1. Advertising
    y u no do it?

    Advertising (learn more)

    Advertise virtually anything here, with CPM banner ads, CPM email ads and CPC contextual links. You can target relevant areas of the site and show ads based on geographical location of the user if you wish.

    Starts at just $1 per CPM or $0.10 per CPC.

$_GET has junk value

Discussion in 'PHP' started by Jeremy Benson, Mar 21, 2015.

  1. #1
    Hello,

    I'm working on a site where I'm encode all the data that goes into my urls and decoding when I need to use it. It was working, but on the listing page of my site the data coming out is junk. It has diamonds with questions marks in them, tilds, not sure what it is.

    I have links like below...
    <p style="margin:4px 0 0 8px;"><a href="postad.php?maincategory='.base64_encode(urlencode('localspots')).'">Local Spots</a></p>
    
    PHP:
    Meta redirect links like below

    
    echo '<META HTTP-EQUIV="Refresh" Content="0; URL=../../../postad.php?maincategory='.base64_encode(urlencode($mianCategory)).'&subcategory='.base64_encode(urlencode($subCategory)).'&minimalcategory='.base64_encode(urlencode($minimalCategory)).'&form=true&jobtitle='.base64_encode(urlencode($jobTitle)).'&jobdescription='.base64_encode(urlencode($jobDescription)).'&jobeducation='.base64_encode(urlencode($jobEducation)).'&jobwages='.base64_encode(urlencode($jobWages)).'&email='.base64_encode(urlencode($email)).'&forwardemails='.base64_encode(urlencode($forwardEmails)).'&video1='.base64_encode(urlencode($video1)).'&video2='.base64_encode(urlencode($video2)).'&video3='.base64_encode(urlencode($video3)).'&image1='.base64_encode(urlencode($image1)).'&image2='.base64_encode(urlencode($image2)).'image3='.base64_encode(urlencode($image3)).'image4='.base64_encode(urlencode($image4)).'&priority='.base64_encode(urlencode($priority)).'&prioritytime='.base64_encode(urlencode($priorityTime)).'&sponsored='.base64_encode(urlencode($sponsored)).'&sponsoredtime='.base64_encode(urlencode($sponsoredTime)).'&textad='.base64_encode(urlencode($textAd)).'&textadtime='.base64_encode(urlencode(textAdTime)).'&payment='.base64_encode(urlencode('true')).'">';
    
    PHP:
    Then when I'm in a php tag I want to get the data in I'll do this...
    
    $mainCategory =  urldecode(base64_decode($_GET['maincategory']));
    $subCategory = urldecode(base64_decode($_GET['subcategory']));
    $minimalCategory = urldecode(base64_decode($_GET['minimalcategory']));
    $form = urldecode(base64_decode($_GET['form']));
    $jobTitle = urldecode(base64_decode($_GET['jobtitle']));
    
    PHP:
    I'm not sure what's making my string come out like this..

    N�� >k��jYr��(��^r�����}���Z�ﭣ�(���i�.��r���~���˫{�a{�(���z�>j�����x>���������{��G����

    any help would be great :)

    Thanks,
    Jeremy.
     
    Jeremy Benson, Mar 21, 2015 IP
  2. deathshadow

    deathshadow Acclaimed Member

    Messages:
    9,732
    Likes Received:
    1,998
    Best Answers:
    253
    Trophy Points:
    515
    #2
    You've got your urlencode and base64encodes backwards.

    You have: base64_encode(urlencode($mianCategory))

    When you should have: urlencode(base64_encode($mianCategory))

    The URL encode is what should be sent to the markup, not the base64. NOT that I would be bothering using base64_encode in the first place if passing get -- it's NOT like you are actually securing a blasted thing with it or that it's providing anything useful.

    Why not just send:

    echo '
    	<meta
    		http-equiv="refresh"
    		Content="0; URL=../../../postad.php?maincategory=', urlencode($mianCategory),
    		'&subcategory=', urlencode($subCategory),
    		'&minimalcategory=', urlencode($minimalCategory),
    		'&form=true&jobtitle=', urlencode($jobTitle),
    		'&jobdescription=', urlencode($jobDescription),
    		'&jobeducation=', urlencode($jobEducation),
    		'&jobwages=', urlencode($jobWages),
    		'&email=', urlencode($email),
    		'&forwardemails=', urlencode($forwardEmails),
    		'&video1=', urlencode($video1),
    		'&video2=', urlencode($video2),
    		'&video3=', urlencode($video3),
    		'&image1=', urlencode($image1),
    		'&image2=', urlencode($image2),
    		'&image3=', urlencode($image3),
    		'&image4=', urlencode($image4),
    		'&priority=', urlencode($priority),
    		'&prioritytime=', urlencode($priorityTime),
    		'&sponsored=', urlencode($sponsored),
    		'&sponsoredtime=', urlencode($sponsoredTime),
    		'&textad=', urlencode($textAd),
    		'&textadtime=', urlencode(textAdTime),
    		'&payment=', urlencode('true'),
    	'">';
    Code (markup):
    So you can just urldecode -- just what are you sending that requires base64? Looks like a NASTY case of over-thinking the problem to me.

    NOT, NOT that would EVARRR send that much getData in a redirect or href -- Holy Hannah! Isn't there some way you could store that server side under a single ID code or session or something?!? Yeah, a session would make WAY more sense so you aren't wasting time sending all that data for NOTHING over the web. It just reeks of sending data client-side that has no business being sent client-side.

    Oh, and when possible, TRY to use comma delimits instead of string additions on echo. It executes faster and uses less memory since the string chunks can be simply output instead of it all having to be added together before being sent. See the classic "this is a test"

    function echoTest() {
    	echo 'test.';
    }
    
    echo 'This is a ', echoTest(); // outputs "This is a test."
    echo 'This is a ' . echoTest(); // outputs "test. This is a"
    Code (markup):
    Which can drive you nutters if you aren't aware of how addition is executed. (see turdpress which calls functions that do exactly this! Output with echo instead of returning a value, but are called inline with output.)
     
    deathshadow, Mar 21, 2015 IP
  3. Jeremy Benson

    Jeremy Benson Well-Known Member

    Messages:
    364
    Likes Received:
    4
    Best Answers:
    0
    Trophy Points:
    123
    #3
    Thanks, I just prefer it. I don't like readable urls other than the website link and folders, lol. The only time I would hide all my variables is if I was making a blog site or article site like news... personal preference I guess... It helps secure a bit.. not from coders, but most people who just use the internet don't even know what a query string is, let alone how to un-encode one, :p
     
    Jeremy Benson, Mar 22, 2015 IP
  4. EricBruggema

    EricBruggema Well-Known Member

    Messages:
    1,740
    Likes Received:
    28
    Best Answers:
    13
    Trophy Points:
    175
    #4
    Jeremy its better to have the urls contain a word or text that's indexable by search engines if you wish they to be found... if not, then this is the correct way :p btw

    Why not walking all get values instead of rebuilding (even non existing) parameters
    foreach ($_GET AS $key=>$val) { $$key = urldecode(base64_decode($val)); }
     
    EricBruggema, Mar 22, 2015 IP
  5. deathshadow

    deathshadow Acclaimed Member

    Messages:
    9,732
    Likes Received:
    1,998
    Best Answers:
    253
    Trophy Points:
    515
    #5
    I would HIGHLY advise against that -- though I SUPPOSE it would depend on what you are doing with it. That someone could BS some getData to make as many variables in your code containing whatever they wanted could REALLY be exploited; from denial of service attacks to code elevation. I mean, what if you had a global like $userID and they bullshitted &userId=0; - mind you putting something like the user ID into a global is dumbass, but enough systems out there have "security, what's that" code, and user 0 is usually admin -- so... crap. Joe help you if someone were to pass something like [user][permissions][deletePost]=1 or some such.

    Though could be worse, could plug $_GET directly into queries that way. SMF had a vulnerability like 8 years ago in the Avatar and attachment codebase because the user information edit page pulled a similar stunt. The problem was one of the user preferences was the theme; the theme selection code was secure, but in the normal user information you could pass it a BS value that would assign the theme path to whatever you wanted -- including avatars or attachments. Net result? Blindly assuming the indexes of $_GET were valid values resulted in being able to run as PHP code anything any user of the forum software could upload.

    But that's PHP for you; insecure by design. That you can even do constructs like the one you posted is proof enough of that. It's actually TERRIFYING how bad/insecure some of the stuff you can do in PHP is -- things like doing a foreach on $_GET for example and then blindly plugging it into new variables by index.

    $$ is useful from time to time, but in a lot of ways it's just BEGGING for someone to come along and give your codebase a nice big dose of BOHICA "eval style".

    NOT that I understand why people seem to insist on copying values out of $_POST or $_GET into pointless memory wasting processing time wasting variables in the first place... or why anyone would send that much data in a href or meta for that matter. I'd sooner eat a bullet than send a URI that long over the web. Again why I'd be tracking all that **** server-side and using a unique identifier at most as what's passed in the URI... since if your URI is going to end up bigger than the plaintext on most PAGES, you're probably doing something horribly and terrifyingly wrong. MUCH LESS how easy it would be to bullshit those values to create exploits.

    Remember, ALL user input is suspect and should be double, triple and quadruple checked. Blindly trusting that getData will be passed accurately; particularly that much raw data in a HREF or META redirect is just BEGGING some cracker to come along and pwn you.

    Best way to avoid that, don't send data client side that doesn't need to be sent client side; though try and convince some of todays "use jquery" AJAX for EVERYTHING scripttards of that one.
     
    Last edited: Mar 23, 2015
    deathshadow, Mar 23, 2015 IP
  6. EricBruggema

    EricBruggema Well-Known Member

    Messages:
    1,740
    Likes Received:
    28
    Best Answers:
    13
    Trophy Points:
    175
    #6
    I agree, but in this case the code is already poorly writen.. (not an excuse ofcource).

    But its not realy bad as you say, as a good programmer will assign variables before using them (not in the 'startup' of the scripting). I used this to remove the slashes from $_POST to prefent injection (not anymore)

    Better version (more secure is)
    
    foreach (array('key', 'key1', 'key2', 'etc') AS $k) {
        if (isset($_GET[$k])) {
            $$k = doyouthinghere($_GET[$k]);
        }
    }
    
    Code (markup):
    And i'm shure that when you are a good scripter you'll assign values to strings if you need them (i always do stuff like this).

    $page = (isset($_GET['page']) && is_numeric($_GET['page'])) ? $_GET['page'] : 0;
     
    EricBruggema, Mar 23, 2015 IP
  7. deathshadow

    deathshadow Acclaimed Member

    Messages:
    9,732
    Likes Received:
    1,998
    Best Answers:
    253
    Trophy Points:
    515
    #7
    A whitelist like that makes a bit more sense and is good practice, but I probably still wouldn't be making new variables or I'd do it when I need the value, not ahead of time... unless that function/processing is so complex and the result used more than once. The only real reason I'd consider that type of processing STILL wouldn't be using $$, I'd be plugging them into an array for PDOStatement::execute()

    I see so much PHP and JS code where people seem to be wasting time making variables for values/results they only then go and use once... go ahead, increase the memory footprint and make garbage collection harder... it's like people never heard of stack overflows or stack thrashing.
     
    deathshadow, Mar 25, 2015 IP
  8. EricBruggema

    EricBruggema Well-Known Member

    Messages:
    1,740
    Likes Received:
    28
    Best Answers:
    13
    Trophy Points:
    175
    #8
    What has stack overflows to do with cleaning up user input?? its better to clean and or validate all the input before starting any database queries... complex functions and such...
     
    EricBruggema, Mar 27, 2015 IP
  9. deathshadow

    deathshadow Acclaimed Member

    Messages:
    9,732
    Likes Received:
    1,998
    Best Answers:
    253
    Trophy Points:
    515
    #9
    Every function call has a extra memory overhead from saving the state, this is usually put into a memory area called "the stack". Local variables typically go? On the stack. Variables passed from one section of code to a function/method? Copied onto the stack.

    That's why variables for nothing is garbage. If the routine being called takes a good long while to run and you need to output the result more than once, go ahead and put it in a variable, but halfwit garbage coding like this:

    
    $userList = $statement->fetchAll();
    foreach ($userList as $user) {
    	$name = htmlspecialchars($user['name']);
    	$address = htmlspecialchars($user['addresss']);
    	$city = htmlspecialchars($user['city']);
    	echo '<tr>
    		<th scope="row">' . $name . '</th>
    		<td>' . $address . '</td>
    		<td>' . $city . '</td>
    	</tr>';
    }
    Code (markup):
    Has three to several thousand times the memory footprint (depending on the size of the result set) of:

    while ($user = $statement->fetch()) {
    	echo '<tr>
    		<th scope="row">', htmlspecialchars($user['name']), '</th>
    		<td>', htmlspecialchars($user['address']), '</td>
    		<td>', htmlspecialchars($user['city']), '</td>
    	</tr>';
    }
    Code (markup):
    Since fetchAll automatically doubles the memory footprint of the result set, the variable copies "for nothing" uses memory for nothing, and the string additions require everything to be evaluated and added together before being sent to the output - yet you see that former code being used ALL THE TIME now... leaving older developers like myself wondering just what the foxtrot whiskey alpha romeo people are smoking!

    Snoop, you'd know better than anybody; what's this brother smoking over here?!?

    Actually helped someone about six months ago who's code kept throwing 500 errors, he was doing the above with a result set that was so large it WAS blowing out the stack limit. Changing it to the latter example ran twenty times faster and reduced the memory footprint 80%.

    <dt>Stack Overflow</dt>
    <dd>Making so many local variables and passing so many parameters you run out of stack memory space.</dd>

    I'm often amazed how many programmers think it's just the name of a website.

    Now mind you, if you had a complex validation function being performed, the code reduction of passing to the function is worth it; if you are using the same result more than once in your output, THEN it's worth using the local var... but if you are only using the result once, DON'T WASTE PROCESSING TIME AND MEMORY ON A VARIABLE!

    Admittedly, I come from a time where we bothered knowing that there's a difference between speed and memory optimization -- and sometimes what's fastest in small doses is painfully slow with larger data sets. The difference between "mov ecx, 10; .loop: movsd; loop .loop" and "movsd; movsd; movsd; movsd; movsd; movsd; movsd; movsd; movsd; movsd;"

    Actually, bad example, more like the difference between mov bx, 80; mul bx; and mov bx, ax; mov ax, [bx + lookupTable] using a 400 byte table on a 8088...
     
    Last edited: Mar 28, 2015
    deathshadow, Mar 28, 2015 IP
  10. EricBruggema

    EricBruggema Well-Known Member

    Messages:
    1,740
    Likes Received:
    28
    Best Answers:
    13
    Trophy Points:
    175
    #10
    Point taken! :)
     
    EricBruggema, Mar 31, 2015 IP