Go to main content

Textpattern CMS support forum

You are not logged in. Register | Login | Help

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

colak
Admin
From: Cyprus
Registered: 2004-11-20
Posts: 9,007
Website GitHub Mastodon Twitter

Re: mem_postmaster - Postmaster Revamp

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.


Yiannis
——————————
NeMe | hblack.art | EMAP | A Sea change | Toolkit of Care
I do my best editing after I click on the submit button.

Offline

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

THE BLUE DRAGON
Member
From: Israel
Registered: 2007-11-16
Posts: 619
Website

Re: mem_postmaster - Postmaster Revamp

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)

Offline

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

THE BLUE DRAGON
Member
From: Israel
Registered: 2007-11-16
Posts: 619
Website

Re: mem_postmaster - Postmaster Revamp

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?

Offline

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

Gocom
Developer Emeritus
From: Helsinki, Finland
Registered: 2006-07-14
Posts: 4,533
Website

Re: mem_postmaster - Postmaster Revamp

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.

Offline

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

THE BLUE DRAGON
Member
From: Israel
Registered: 2007-11-16
Posts: 619
Website

Re: mem_postmaster - Postmaster Revamp

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.

Offline

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

Gocom
Developer Emeritus
From: Helsinki, Finland
Registered: 2006-07-14
Posts: 4,533
Website

Re: mem_postmaster - Postmaster Revamp

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)

Offline

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

THE BLUE DRAGON
Member
From: Israel
Registered: 2007-11-16
Posts: 619
Website

Re: mem_postmaster - Postmaster Revamp

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 ;)

Offline

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

colak
Admin
From: Cyprus
Registered: 2004-11-20
Posts: 9,007
Website GitHub Mastodon Twitter

Re: mem_postmaster - Postmaster Revamp

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?


Yiannis
——————————
NeMe | hblack.art | EMAP | A Sea change | Toolkit of Care
I do my best editing after I click on the submit button.

Offline

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

craigdrown
Member
Registered: 2006-01-02
Posts: 19

Re: mem_postmaster - Postmaster Revamp

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?

Offline

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

jens31
Plugin Author
From: munich / dtschermani
Registered: 2008-08-25
Posts: 183
Website

Re: mem_postmaster - Postmaster Revamp

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?

Offline

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

THE BLUE DRAGON
Member
From: Israel
Registered: 2007-11-16
Posts: 619
Website

Re: mem_postmaster - Postmaster Revamp

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.

Offline

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

jens31
Plugin Author
From: munich / dtschermani
Registered: 2008-08-25
Posts: 183
Website

Re: mem_postmaster - Postmaster Revamp

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..

Offline

Board footer

Powered by FluxBB