GET parameters are parsed incorrectly when base64 string is passed through GET

[mumnik]

Latest core update disabled some my one process. While investigating the issue I've discovered that $_GET is being overwritten in Input_Instance class, uri(). When param contains + or may be some other special characters parse_str performs incorrect by replacing + with space. For now i've disabled GET overwriting but guess this needs to be fixed.

[WanWizard]

You need to urlencode() your URL parameters, a + is not allowed in a URL as it is the placeholder for a space.

[mumnik]

Params are coming urlencoded

[WanWizard]

If that is the cause, then a + is correctly translated into a space.

If not, please provide an example of a URL.

[mumnik]

I have to hide some parts because there is sensitive data, bet the problem part is unchanged, it's the "VK_MAC" param GET /site/cart/callback?VK_SERVICE=1101&VK_VERSION=008&VK_SND_ID=HP&VK_REC_ID=FRT&VK_STAMP=131239&VK_T_NO=279&VK_AMOUNT=21.4&VK_CURR=EUR&VK_REC_ACC=LV41HABA0551035980071&VK_REC_NAME=TECHNOLOGY+SIA&VK_SND_ACC=LV&VK_SND_NAME=KIS&VK_REF=131239&VK_MSG=p.l%2C+Pas%C5%ABt%C4%ABjuma+numurs%3A+00131239&VK_T_DATE=25.11.2018&VK_LANG=LAT&VK_AUTO=Y&VK_MAC=Hjr3N%2Fuwt27m9Sy0ncdECEu%2FyJycWoZXEH%2FX4vdkz%2Bs9jaG1cvK56SgEZ%2BB1EqJ2bShX4v%2FkG%2BYG4UgJWOAqMKTukvmeMLouV1CQ0GParFguu81THO7YVhe0PZxEKVxmPdI6pVm35c%2F99d8acDipznrruZOAH8d7S7ta00G5jNU%3D&VK_ENCODING=UTF-8

[WanWizard]

That decodes to

array(19) {
  'VK_SERVICE' =>
  string(4) "1101"
  'VK_VERSION' =>
  string(3) "008"
  'VK_SND_ID' =>
  string(2) "HP"
  'VK_REC_ID' =>
  string(3) "FRT"
  'VK_STAMP' =>
  string(6) "131239"
  'VK_T_NO' =>
  string(3) "279"
  'VK_AMOUNT' =>
  string(4) "21.4"
  'VK_CURR' =>
  string(3) "EUR"
  'VK_REC_ACC' =>
  string(21) "LV41HABA0551035980071"
  'VK_REC_NAME' =>
  string(14) "TECHNOLOGY SIA"
  'VK_SND_ACC' =>
  string(2) "LV"
  'VK_SND_NAME' =>
  string(3) "KIS"
  'VK_REF' =>
  string(6) "131239"
  'VK_MSG' =>
  string(34) "p.l, Pasūtījuma numurs: 00131239"
  'VK_T_DATE' =>
  string(10) "25.11.2018"
  'VK_LANG' =>
  string(3) "LAT"
  'VK_AUTO' =>
  string(1) "Y"
  'VK_MAC' =>
  string(172) "Hjr3N/uwt27m9Sy0ncdECEu/yJycWoZXEH/X4vdkz+s9jaG1cvK56SgEZ+B1EqJ2bShX4v/kG+YG4UgJWOAqMKTukvmeMLouV1CQ0GParFguu81THO7YVhe0PZxEKVxmPdI6pVm35c/99d8acDipznrruZOAH8d7S7ta00G5jNU="
  'VK_ENCODING' =>
  string(5) "UTF-8"
}

which look fine to me?

[mumnik]

somehow i get "Hjr3N/uwt27m9Sy0ncdECEu/yJycWoZXEH/X4vdkz s9jaG1cvK56SgEZ B1EqJ2bShX4v/kG YG4UgJWOAqMKTukvmeMLouV1CQ0GParFguu81THO7YVhe0PZxEKVxmPdI6pVm35c/99d8acDipznrruZOAH8d7S7ta00G5jNU=" instead

[WanWizard]

That smells like double decoding. I'll see if I can find the time to create a small debug app tomorrow.

[mumnik]

i bet it happens here:

if (strpos($uri, '?') !== false)
		{
			// log this issue
			\Log::write(\Fuel::L_DEBUG, 'Your rewrite rules are incorrect, change "index.php?/$1 [QSA,L]" to "index.php/$1 [L]"!');

			// reset $_GET
			//$_GET = array();

			// lets split the URI up in case it contains a ?.  This would
			// indicate the server requires 'index.php?'
			preg_match('#(.*?)\?(.*)#i', $uri, $matches);

			// If there are matches then lets set everything correctly
			if ( ! empty($matches))
			{
				// first bit is the real uri
				$uri = $matches[1];

				// second bit is the real query string
				$_SERVER['QUERY_STRING'] = $matches[2];
			//	parse_str($matches[2], $_GET);

				// update GET variables
				$_GET = \Security::clean($_GET);

				$this->input_get = $_GET;
			}

		}

You can see $_GET overwriting commented out

[WanWizard]

Contents of $_GET is always url decoded, so that's fine. Do you you manually decode again in your app?

[mumnik]

no, i event put debug directly in this segment to check

[WanWizard]

I can reproduce it, how about:

			// lets split the URI up in case it contains a ?.  This would
			// indicate the server requires 'index.php?'
			preg_match('#(.*?)\?(.*)#i', $uri, $matches);

			// If there are matches then lets set everything correctly
			if ( ! empty($matches))
			{
				// first bit is the real uri
				$uri = $matches[1];

				// second bit is the real query string
				$_SERVER['QUERY_STRING'] = urlencode($matches[2]);

				// update GET variables
				parse_str($_SERVER['QUERY_STRING'], $_GET);
				$_GET = \Security::clean($_GET);
				$this->input_get = $_GET;
			}

let me know if this fixes it for you?

[nickhagen]

I am having this problem as well it would seem, here is an example parameter:

url?Auth=DBUW1Dc%2bKwPn7eE%2bZuxzg2fnlww%3d

It seems to be properly URL Encoded when I receive it, however, Input::get('Auth') returns:

AUTH: DBUW1Dc KwPn7eE Zuxzg2fnlww=

So it seems to be double URL decoding somehow.

I have tried your last suggestion on November 27th, but that did not fix the issue. Ultimately since I know this is a BASE64 encoded parameter, I am using:

$auth = str_replace(' ', '+', Input::get('Auth', 0));

But that won't work in any other case.

[WanWizard]

@nickhagen can you try this?

It handles both your example and the one above without issues here.

[WanWizard]

p.s. (and that is to all), it might be a better idea to do that the DEBUG log entry at the beginning of that code bit says, and fix your rewrite rule, as that code is only executed if your rewriting is wrong.

[nickhagen]

Well ... the plus signs are definitely correctly interpreted now, however, the equal sign at the end is still url encoded:

DBUW1Dc+KwPn7eE+Zuxzg2fnlww%3D

I tried updating my htaccess file in my public directory to change the rewrite rules as suggested but that didn't seem to change anything.

RewriteRule ^(.*)$ index.php?/$1 [L]

[WanWizard]

Great. Not.

The problem is that in the QUERY_STRING, the first "=" is also URL encoded. I don't know if this is a bug in PHP or something that has changed, but it didn't used to be the case.

Back to the drawing board...

As to the rewrite rule, if yu use the default htaccess, you have to make sure you change the correct rewrite for your PHP version and type.

[WanWizard]

Attempt two.

I used this URL for testing:

http://19develop.dev/welcome/index/a%20space/a+plus/?Auth=DBUW1Dc%2bKwPn7eE%2bZuxzg2fnlww%3d&date=2013-10-17T22%3A09%3A24%2B00%3A00

resulting in thse URI segments:

array (size=4)
  0 => string 'welcome' (length=7)
  1 => string 'index' (length=5)
  2 => string 'a space' (length=7)
  3 => string 'a+plus' (length=6)

and these arguments:

array (size=3)
  'Auth' => string 'DBUW1Dc+KwPn7eE+Zuxzg2fnlww=' (length=28)
  'date' => string '2013-10-17T22:09:24+00:00' (length=25)

[nickhagen]

BAM! That seems to fix it, are there any potential problems that I should look out for with removing that processing line?

[WanWizard]

It shouldn't. I went through the commit history, appearently the decoding/encoding was added to address some nginx quirk. But the examples given are handled buy this code as well.

I'm closing this issue, let me know if you still find issues.