Quirky url handling

I posted this as a bug (http://bugs.modx.com/browse/MODX-2306), but thought I would post it here too for discussion on how it should work....

Revolution handles friendly urls in a funny way. You can type in crazy urls, and it will serve a "random" page rather than a 404 IF the first few characters are a number that is a valid, published resource id.

Example:
http://www.mysite.com/4I-can-typeANYTHING_IwantTO-afterthenumber.whatever

Will load the resource with id 4. My initial thought is that if the "q" url parameter does not contain a valid _alias_ a 404 should be returned (requests for a specific resource by id should use the id url param).

I suppose this could potentially be used to exploit some poorly written snippet code as well?

What are others thoughts on how this should work?

Interesting find, rfoster.... my first reaction is that the regex on alias matching is off, but I’ll defer to Shaun and Jason.

Essentially what is happening is that if the q parameter doesn’t match any alias, it is assumed that it is an id. When grabbing the resource intval() is used, which will return a number as long as the first character is a number.

This could be bad. Imagine a bunch of people link to your site, and say anything they want about it...
http://modxcms.com/1-reason-I-just-LOVE-MODx-Revolution.html

..and Google comes along and lists that in their search results. Definitely could lead to some SEO abuse of your site.

Did you file this as a bug?

Is it also happening when you precise a 404 document landing in the settings (linked to a proper 404 resource) ?

Quote from: lossendae at Aug 27, 2010, 02:50 AM

Is it also happening when you precise a 404 document landing in the settings (linked to a proper 404 resource) ?

Yes it is. And it *is* filed as a bug.

Try changing core/model/modx/modrequestclass.inc.php line 145 from:



to

I think it should be stricter than that. Is numeric also allows a wire range of input.

bool is_numeric ( mixed $var )
Finds whether the given variable is numeric. Numeric strings consist of optional sign, any number of digits, optional decimal part and optional exponential part. Thus +0123.45e6 is a valid numeric value. Hexadecimal notation (0xFF) is allowed too but only without sign, decimal and exponential part.

Better would be something like:

is_numeric() at least kills the obvious abuse cases where strings are appended after a number. I’m not sure it’s worth doing more because it would slow things down some since it will be executed on every page request.

A strlen() check should also do it.