search

JamesHeinrich / phpThumb

phpThumb() - The PHP thumbnail generator
Other
315 stars 98 forks source link

Old Safari bug with $_SERVER['QUERY_STRING'] #122

Closed badbadmonkey closed 6 years ago

badbadmonkey commented 6 years ago

Current version phpThumb.php line 200

elseif ($_GET['hash'] != md5(str_replace($phpThumb->config_high_security_url_separator.'hash='.$_GET['hash'], '', $_SERVER['QUERY_STRING']).$phpThumb->config_high_security_password))

Years ago I had hacked this to urldecode() QUERY_STRING ...

elseif ($_GET['hash'] != md5(str_replace($phpThumb->config_high_security_url_separator.'hash='.$_GET['hash'], '', urldecode($_SERVER['QUERY_STRING'])).$phpThumb->config_high_security_password))

... because of a weird bug I was seeing on Safari at the time to do with the provided value of QUERY_STRING from the browser. It's probably not a bug in newer versions but is there any harm in leaving it in?

Edit: The above breaks current version with an error that seems to relate to invalid hash, but I'll leave this here as a query in case the old Safari versions are still affected.

JamesHeinrich commented 6 years ago

Based on what you wrote above it just looks like a one-off bug in some version(s) of Safari that breaks things. If it works in current version of Safari, and current versions of all other browsers, then I would leave it alone. If for some reason you absolutely need to support said broken version of Safari, I suppose you could include the double-if to check if either the urldecoded or nonurldecoded version matches, but I don't think I'll be making any such change in the codebase.