Security Problem if not Sanitizing Data before Saving by Xpdo to a Database?

Can anybody of the modx-team tell if saving content to a database by xpdo without cleaning the data before saving is a security problem in MODX Revolution regarding injections like it is when using 'mysql_query insert' without previously cleaning the data by mysql_real_escape_string()?

In this thread 'goldsky' strongly recommends to sanitize all data before transferring the data into a database, but I am not sure if this is right:

https://forums.modx.com/thread/?thread=20171&page=1

SQL injection is still possible with XPDO. I ran into a case awhile back where SELECT queries using IN() were vulnerable (or at least produced a SQL error. IN() was expecting numerical values from a snippet property, and the property got the values from a GET request variable with comma-separated list of IDs. Bad idea - the values were not escaped by XPDO, and they weren't sanitized in the snippet either.

Personally, I always sanitize my input variables in every script I write. Every $_* variable is automatically cleaned with HTMLPurifier or converted into HTML entities, trimmed for whitespace, "../", "`", "[[" and "]]" tags are encoded to prevent abuse in the web context. I manually convert numerics to integers and make sure they're within the expected range. If it's a string and I know the possible values, I compare it against an array of expected values, and set it to a default if it's invalid.

This is all before being used in a query, or being displayed to a user.

There's more, but I think you can get the gist of it. I'm paranoid when it comes to code, because I've found that if you're not, it'll find a way to bite you in the backside.

The MODX request handler calls modX::sanitize() on every request to sanitize $_POST, $_GET, and _$COOKIE.

Hi BobRay,

do you know in which cases modX::sanitize() comes into action exactly?

For test-reason I created a formit-form containing the fields [[!+fi.firstname:htmlent]] and [[!+fi.name:htmlent]] which are passed to another page by [[!FormItRetriever]] containing 'Hello [[!+fi.firstname:htmlent]] [[!+fi.name:htmlent]]'.

In first field I typed 'Jeff' the second field ([[!+fi.name:htmlent]]) I typed [[!+fi.firstname]] and the result on the second page is: 'Hello Jeff Jeff'.

I tried the same for snippets (typed in the field [[!mysnippet]] an the processed snippet was given out.

I think this behavior can be a big security problem if you are not aware of this behavior.

For test-reason I also checked what happens when you transfer the values of this test-formit-form into a database by formit2db and in both attempts the modx code was without any escaping written into the database ('[[!+fi.firstname]]' and the same with '[[!mysnippet]]').

I am not sure if this is behavior might be intended on purpose by the modx-team e. g. to give the possibility to write modx-code into databases by forms but it is a critical security issue if you use formit2db and db2formit and you are not aware of this problem.

Do you think I am right in these points or are there other points to take into account which I might not see?

Quote from: wingnutty at Feb 01, 2013, 09:59 PM

Personally, I always sanitize my input variables in every script I write. Every $_* variable is automatically cleaned with HTMLPurifier or converted into HTML entities, trimmed for whitespace, "../", "`", "[[" and "]]" tags are encoded to prevent abuse in the web context.

Hi Rick, very interesting. What do you use for sanitizing/deleting '[[' and ']]'? Do you use an standard output filter of modx (http://rtfm.modx.com/display/revolution20/Input+and+Output+Filters+(Output+Modifiers)) for this or have you done your own custom snippet used for example in this way: [[+inputvalue:myownsnippetforsanitzing]]

In all of my components, I have an "InputCleaner" class which handles sanitization of all $_* variables. So one off the first things that I do in any snippet is a call to the class method which recursively cleans every request variable so that it's safe for display or use.

My replacements are something like this:

@germand: I wrote you a reply, but it seems to have been lost. MODX sanitize() won't necessarily remove MODX tags. It only does so if told to. As of 2.2.6, the allow_tags_in_post System Setting is false by default. I suspect that the tags in your test would not have gotten through under 2.2.6.

That said, there is nothing dangerous about MODX tags in the DB. The only danger is when you display pages with those tags in them to the general public.

I haven't looked at the code to FormIt2DB or DB2FormIt for a long time so I can't speak to any security issues with them.

Quote from: wingnutty at Feb 03, 2013, 02:32 PM

I could post my cleaner class if you'd like.

That would be very nice of you. Maybe we then will use the same technique for sanitizing ...

Thanks again ... [ed. note: gemand last edited this post 13 years, 7 months ago.]

Sure, I've attached my cleaner class to this post. Just use it in your snippet with a call to

InputCleaner::cleanInput( [ bool $allowHTML = false ] )

If you set allowHTML to true, HTMLPurifier (htmlpurifier.org) will sanitize the HTML, remove scripts, and make sure the HTML code is valid. Otherwise, all HTML tags are automatically encoded using htmlentities. This is of course on top of the sanitizing I mentioned earlier for MODX tags.

HTMLPurifier should be located at:

MODX_CORE_PATH/components/htmlpurifier/

Any questions, just ask.

We have similar questions concerning the securitiy of formit-forms and would be very glad if anybody could help us to clarify the following
point:

We found that in formit-forms the following user-input is stored without any escaping in our database:

input --> stored
=================
\x00 --> \x00
\n --> \n
\r --> \r
\ --> \
\x1a --> \x1a

Seems that there is no escaping and we are worried if this could be a risk for mysql-injections?

Should formit2db therefore be updated with something like "mysql_real_escape_string"?