No response body when null-byte character in query string (%00) #13289

Open
pixelchutes opened this Issue Feb 13, 2017 · 5 comments

Projects

None yet

3 participants

@pixelchutes
Collaborator

Summary

If passing a null-byte character (%00) in the URL to a MODX website, the request is immediately terminated with a HTTP/1.1 200 OK response, but no response body.

Step to reproduce

Browse to MODX website with null-byte character in the URL:

Example: https://modx.com/?%00

Observed behavior

HTTP/1.1 200 OK
Connection: keep-alive
Content-Encoding: gzip
Content-Type: text/html; charset=UTF-8
Date: Mon, 13 Feb 2017 21:14:51 GMT
MC: 2Mru1ZFR/JMsNYMAR6VMFC5F3zNmMHNF7EfTrM+JyGvvVVA7Bk+CJhC8hu17ZySp
Server: nginx/1.10.0 (Ubuntu)
Transfer-Encoding: chunked
Vary: Accept-Encoding

// no response body here

Example: https://modxcloud.com/?%00

HTTP/1.1 200 OK
Connection: keep-alive
Content-Encoding: gzip
Content-Type: text/html; charset=UTF-8
Date: Mon, 13 Feb 2017 21:19:33 GMT
MC: RdPkGQARO+eA54v8wZoGfXPmoazL4Uc8YT/K60fe+99+jz1EZWVvKbCnpATyRlGa
Server: nginx/1.8.1
Strict-Transport-Security: max-age=31536000;
Transfer-Encoding: chunked

// no response body here

Testing this against the exact same server (but bypassing MODX entirely) suggests that this is an internal handling issue within MODX core.

Expected behavior

I would expect some accompanying response body if the URI was valid, and if not, then perhaps the 404 Error page, for example. Something more than just the HTTP headers.

Environment

MODX 2.5.5, PHP 5.6.30, NGINX 1.8.* or 1.10.* or 1.11.*

@Mark-H
Collaborator
Mark-H commented Feb 13, 2017 edited

Here's the cause. Seems to have been there since the first revo commit.

Weird to just die() the process though, I can find some resources that say null byte injection can be a security risk, but it seems a simple str_replace on the values (perhaps in modX::sanitize or modX::protect should do the trick?

@pixelchutes
Collaborator

Interesting, good find @Mark-H. I did confirm this against an Evo instance, as well (same results) -- so probably originated there.

Yes, I definitely don't want to make things any less secure ๐Ÿ˜‰ but the handling as-is seems a bit abrupt.

@Mark-H
Collaborator
Mark-H commented Feb 13, 2017

If we want to replace it with a str_replace, I think we'll have to place that into modX::protect - modX::sanitize isn't called until much later compared to where it halts now.

Alternatively, if we want to keep it, we could add a header (400 or 422 perhaps) with a short text description in the existing conditional.

@pixelchutes
Collaborator

I would personally lean towards replacing (sanitizing/cleansing), allowing the traffic to continue through to the site. To me, this seems like a better end result, assuming same protection would effectively still be in place vs. halting the entire request.

Devil's advocate: Is there a reason it wasn't simply replaced originally? Perhaps Evo didn't have as good controls around protection at the time maybe?

@mrhaw
mrhaw commented Feb 13, 2017

Very interesting find! Wonder if it can be exploited in any sense. http://security.stackexchange.com/questions/48187/null-byte-injection-on-php/74660#74660

@pixelchutes pixelchutes changed the title from No response body from MODX sites when null-byte character in request (%00) to No response body when null-byte character in query string (%00) Feb 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment