mem_postmaster - Postmaster Revamp

colak · 2012-07-03 11:42:04

HI Julián, the info might be needed if the subscription form resides in some site other to the one the postmaster is installed. Re security, I have no idea, you might be absolutely right.

THE BLUE DRAGON · 2012-07-03 19:30:32

maniqui wrote:

Nice tip, Gil.

Two comments:

1) I don’t think you need to connect again to the DB, so I think you could get rid of this:

Cool edited my tip code :)
I had that such of feeling that we don’t need to connect again to the database.

maniqui wrote:

2) I wouldn’t want to trigger any kind of panic, but I think there may be a security issue with the way you use the variable ‘uid’ directly in the SQL statement, without any kind of sanitizing. Maybe you could drag Gocom’s (the de-facto security expert on TXP forums) attention and he could confirm if the snippet has some security drawbacks.

Thanks I edited my code and replaced

	$variable['uid'] = $_GET["uid"];

with this

	if($_GET["uid"] != '' && strlen($_GET["uid"]) == 32){
		$variable['uid'] = $_GET['uid'];
	}

That’s all what I personally know to do,
I will contact Gocom :)

Last edited by THE BLUE DRAGON (2012-07-03 20:32:00)

THE BLUE DRAGON · 2012-07-03 20:32:28

maniqui wrote:

2) I wouldn’t want to trigger any kind of panic, but I think there may be a security issue with the way you use the variable ‘uid’ directly in the SQL statement, without any kind of sanitizing.

OK I made a quick search and found that I should use mysql_real_escape_string

So I changed my code from:

		$strSQL = "SELECT * FROM bab_pm_subscribers WHERE unsubscribeID = '".$variable['uid']."'";
		$result = mysql_query($strSQL);

to:

		$strSQL = sprintf("SELECT * FROM bab_pm_subscribers WHERE unsubscribeID='%s'",
			mysql_real_escape_string($variable['uid']));
		$result = mysql_query($strSQL);

Better? did I used it right?

Gocom · 2012-07-03 20:43:23

THE BLUE DRAGON wrote:

I will contact Gocom :)

Julián is correct, that code is very dangerous. If you are using it, please note that you may be in great danger, and so is anyone else that took the tip and is currently using it. I hope there isn’t much of users.

There are few issues, and few in addition to SQL ones Julián mentioned. List includes, but may not be limited to:

The SQL queries are not sanitized. Basically you can request a page as http://example.com/?uid=anySQLqueryhere which can be used to steal content or just delete the website.
It connects to database directly accessing mysql_* functions and either specifies a link. According to what Julián said, it previously it also created a new link which made the situation even worst. When doing a multiple connections to database with, essentially, different charsets, an attacker might be able to inject the queries by using charset injecting and bypass valid sanitation/escaping even if it were done — as the sanitizer expects a different charset.
It picks data from the bab_pm_subscribers table and throws it to the page template without cleaning it first. An attacker can register to the newsletter by using a name as <txp:php> safe_delete('textpattern', '1=1'); </txp:php>. As the data isn’t sanitized, the code used as name is ran when the user’s info page (http://example.com/?uid=myinfopage) is accessed and all of the site’s articles are now gone.
The data is also used in a JavaScript. This means that in addition to cleaning the data from TXP related code, it also needs to be sanitized for JavaScript. Currently an attacker can perform an XSS attack and inject any JavaScript to the page too.

Basically what should be done (at least):

Instead of $_GET, Textpattern’s gps() function should be used.
mysql_ function should be replaced with Textpattern DB functions. I.e. safe_row('field1, field2', 'table', 'statement')
Values used in queries should be prepared with doSlash() to avoid SQL injections.
Values added to variables should be escaped with htmlspecialchars().
Values used in JavaScript should be escaped with escape_js().
Values used directly in jQuery’s method’s, $.post(), URL param should be prepared with PHP’s urlencode(), or passed with the data param instead.

THE BLUE DRAGON · 2012-07-03 21:51:25

Thanks that is great Jukka!
I removed the code for now from my tip post, and sent you the new one to check it before I will post it here.

Gocom · 2012-07-03 22:33:21

The code you sent to me mixes JavaScript and PHP, which doesn’t work. But it did fix the MySQL injection issues. Honestly I’m not terribly interested in all this, but here goes some quickie:

<txp:php>

	global $variable;

	$uid = gps('uid');

	if(!$uid) {
		return;
	}

	$r = safe_row(
		'*',
		'bab_pm_subscribers',
		"unsubscribeID='".doSlash($uid)."'"
	);

	foreach($r as $name => $value) {
		$variable[$name] = htmlspecialchars($value);
		$variable['js_'.$name] = escape_js($variable[$name]);
	}

</txp:php>

<txp:variable name="unsubscribeID">

	<txp:variable name="subscriberFirstName" />
	<txp:variable name="subscriberLastName" />
	<txp:variable name="subscriberEmai" />
	<txp:variable name="subscriberCustom1" />
	<txp:variable name="subscriberCustom2" />

	<script>
		$.post(
			"url",
			{
				"formkey" : "dDE3423cnfjWG6tWE2ghRWluWhgfxE6u3Q",
				"value1" : "<txp:variable name="js_subscriberFirstName" />",
				"value2" : "<txp:variable name="js_subscriberLastName" />"
			}
		);
	</script>

<txp:else />
	<txp:txp_die status="404" />
</txp:variable>

Does a query. If row is found, loops through the given array map and generates two set of variables named after the column names (ugh, those are long). One for HTML use and second for JavaScript, prefixed with js_ prefix.

Since variable tag can do a isset check, the template markup code is wrapped in it. In the else statement <txp:txp_die /> is used to return 404 page when a subscriber with requested uid doesn’t exist.

Last edited by Gocom (2012-07-03 22:36:30)

THE BLUE DRAGON · 2012-07-04 07:45:24

Now that is indeed pretty stuff :)
Thanks!

There’s only a tiny mistake in your code that need to be fix by changing:
<txp:variable name="unsubscribeID">
</txp:variable>
to:
<txp:if_variable name="unsubscribeID">
</txp:if_variable>

It was late ;)

colak · 2012-09-02 08:48:29

mem_postmaster does not send any emails with the latest txp v4.5. Michael do you have any plans to upgrade the plugin?

Alternatively, is there anybody who could help make this plugin behave again or even adopt it?

craigdrown · 2012-10-04 00:20:14

Same here: happy to contribute $100 to the person who can get this working- thanks.

colak wrote:

mem_postmaster does not send any emails with the latest txp v4.5. Michael do you have any plans to upgrade the plugin?

Alternatively, is there anybody who could help make this plugin behave again or even adopt it?

jens31 · 2012-11-08 09:40:33

hi

i have this tool running on a shared host, but since a month or so, the provider blocks the sending and recommends to use the smtp server for sending mails. is there a chance to mod this tool for sending mails via smtp?
i have no clue about the technical difficulties, if its possible and what not..

what do you think?

THE BLUE DRAGON · 2012-11-15 15:02:58

jens31 wrote:

is there a chance to mod this tool for sending mails via smtp?

Interesting,
I never tried it, but you will need to change your $headers and mail() function, which are:

	// set header variables, so that only happens once

	$headers = "............

and the mail function:

mail($subscriberEmail, $subject, $email, $headers);

I don’t know exactly how, but there is a sample code here that may help you.

If you do made it, please share how with us.

jens31 · 2012-11-25 17:24:47

thanks gil, your hints helped a lot.

…i got this working right now but it was a trail and error trip and im still not sure if everything is tight now. doesnt feel too good.
like i had to skip the mime type thing, since i really dont know how to handle this.

the postmaster_library overwrites the $header so you have to take care of this as well.
guess some guy smarter as me should melt this stuff into one file and release a new plugin..

Textpattern CMS

Textpattern CMS support forum

#241 2012-07-03 11:42:04

Re: mem_postmaster - Postmaster Revamp

#242 2012-07-03 19:30:32

Re: mem_postmaster - Postmaster Revamp

#243 2012-07-03 20:32:28

Re: mem_postmaster - Postmaster Revamp

#244 2012-07-03 20:43:23

Re: mem_postmaster - Postmaster Revamp

#245 2012-07-03 21:51:25

Re: mem_postmaster - Postmaster Revamp

#246 2012-07-03 22:33:21

Re: mem_postmaster - Postmaster Revamp

#247 2012-07-04 07:45:24

Re: mem_postmaster - Postmaster Revamp

#248 2012-09-02 08:48:29

Re: mem_postmaster - Postmaster Revamp

#249 2012-10-04 00:20:14

Re: mem_postmaster - Postmaster Revamp

#250 2012-11-08 09:40:33

Re: mem_postmaster - Postmaster Revamp

#251 2012-11-15 15:02:58

Re: mem_postmaster - Postmaster Revamp

#252 2012-11-25 17:24:47

Re: mem_postmaster - Postmaster Revamp

Board footer