Icy Phoenix

     
 


Post new topic  Reply to topic 
Page 1 of 1
 
 
Reply with quote Download Post 
Post You Do Not Sanitize Data In $_GET 
 
From everything that I learned about phpBB and Icy Phoenix there seems to be a LOT of emphasis on sanitizing  _GET?
for security reasons.

Well it's driven me MAD!  

The "Quoted" text is saying that the data should be in plain form UNLESS it is being embedded into a NEW string?

Quote:
You do not sanitize data in $_GET. This is a common approach in PHP scripts, but it's completely wrong*.

All your variables should stay in plain text form until the point when you embed them in another type of string. There is no one form of escaping or ‘sanitization’ that can cover all possible types of string you might be embedding your values into.

So if you're embedding a string into an SQL query, you need to escape it on the way out:

http://stackoverflow.com/questions/...a-in-get-by-php

So on the surface it appears that this is fine without "sanitizing it" ???

Code: [Download] [Hide] [Select]
$game_name = $_GET['name'];


And it's driving me nuts because this ->

function blah!

Code: [Download] [Hide] [Select]
$output = mysql_real_escape_string($input);

Adds a slash

And without this AFTER mysql_real_escape_string ->

Code: [Download] [Hide] [Select]
$output = stripslashes($output);


One can't get rid of the slashes before the ' or " etc?

Unless one adds stripslahes to every var that contains text like ->

Code: [Download] [Hide] [Select]
$game_name = stripslahes($_GET['name']);


So! Is it better to add it in the "Sanitize" function or add it to each and every $VAR that handles text?

I GIVE UP!  

Edit:
I thought it may be wise to add the function I'm using.

Code: [Download] [Hide] [Select]
// Do a general clean up of POST and GET.

// Function for stripping out malicious bits
function cleanInput($input) {
$search = array(
'@<script[^>]*?>.*?</script>@si', // Strip out javascript
'@<[\/\!]*?[^<>]*?>@si', // Strip out HTML tags
'@<style[^>]*?>.*?</style>@siU', // Strip style tags properly
'@<![\s\S]*?--[ \t\n\r]*>@' // Strip multi-line comments
);
$output = preg_replace($search, '', $input);
return $output;
}
// Sanitization function
function sanitize($input) {
if (is_array($input)) {
foreach($input as $var=>$val) {
$output[$var] = sanitize($val);
}
}
else {
if (get_magic_quotes_gpc()) {
$input = stripslashes($input);
}
$input = cleanInput($input);
$output = mysql_real_escape_string($input);
}

$output = stripslashes($output);

return $output;
}

 



 
mortSend private message  
Back to topPage bottom
Icy Phoenix is an open source project, you can show your appreciation and support future development by donating to the project.

Support us
 
Reply with quote Download Post 
Post Re: You Do Not Sanitize Data In $_GET 
 
I'd recommand against using your own strip_tags function. Use php's, it's better (=> safer).

magic quotes have been removed on the language recently (5.4 I think?) so it should be safe to remove it now (or soon-ish).

For the rest, I agree: don't preemptively quote stuff. Your template engine should be responsible for the HTML escaping, and your DB layer should be responsible for the sql escaping (two things that phpbb doesn't do...).
Very simple reason:
 - If you pre-filter everything, it's going to be a pain to get the original (sometimes impossible)
 - If you filter "by hand" (applying a function to sanitize everytime) you're going to miss some of those and create vulnerabilities in your software.
 




____________
IcyPhoenix ADR RPGEzArena (modded phpBB2+ADR)
 
InformproSend private message  
Back to topPage bottom
Reply with quote Download Post 
Post Re: You Do Not Sanitize Data In $_GET 
 
Thanks Mate,

I understand that what you are saying is to put the data into the DB as raw data - such as ' and " and then clean it up with the html entities, specialchars, _decode etc when it's requested?

Sounds easy - But it's giving me a headache trying to get it all to work even though I've removed all the htmlentities from the DB and treat them on request with html

I have a lot of files that have spaces in the html "name" in url's like <a href="blah" title="The little Red Hen"></a> which I fill with string replace " " with %20 so that there are no html errors.

Now I can't even get that to work any more?  

And it started because of this ->

Code: [Download] [Hide] [Select]
function sanitize($val){

    $val = trim($val);

    if(get_magic_quotes_gpc()) {
    $val = stripslashes($val);
    }
    $val = strip_tags($val);
    $val = htmlspecialchars($val, ENT_QUOTES,'UTF-8');

    return mysql_real_escape_string($val);
}


I was advised by someone who's fairly professional that htmlentities shouldn't be used in the DB and I should remove it from this function..

If you agree - I'll pursue all of the problems that it's created - If you don't - I'll put it back the way it was.  
 



 
mortSend private message  
Back to topPage bottom
Reply with quote Download Post 
Post Re: You Do Not Sanitize Data In $_GET 
 
I agree with him. You shouldn't store html-safe content (except, obviously, if it's for performance reason). It should only be escaped when needed. And the kind of escaping will differ if it's "in the page", as an attribute, etc...Though problem!
 




____________
IcyPhoenix ADR RPGEzArena (modded phpBB2+ADR)
 
InformproSend private message  
Back to topPage bottom
Reply with quote Download Post 
Post Re: You Do Not Sanitize Data In $_GET 
 
Thanks fella,

Because you both convinced me that what I've been doing in the past is just following a bad example and working hard to keep it fudged and patched to get an outcome.

Anyway, I've sorted out all the front end and there's no script or html errors or warnings.  

It's also a LOT easier too to do it the right way the first time.

Here's my new "sanitise" function that I didn't want to overkill!  

Code: [Download] [Hide] [Select]
Do a general clean up of POST - GET can look after itself when it's called on to do something. :)

function sanitise($val){

    $val = trim($val);

    return mysql_real_escape_string($val);
}


Tomorrow the back end, and then a few more questions here and at the devshed...

BOOM!  
 



 
mortSend private message  
Back to topPage bottom
Reply with quote Download Post 
Post Re: You Do Not Sanitize Data In $_GET 
 
mort wrote: [View Post]
It's also a LOT easier too to do it the right way the first time.

Only after few years struggling you really realize the power of coding functions and classes...
 




____________
Luca
SEARCH is the quickest way to get support.
Icy Phoenix ColorizeIt - CustomIcy - HON
 
Mighty GorgonSend private messageSend e-mail to userVisit poster's website  
Back to topPage bottom
Reply with quote Download Post 
Post Re: You Do Not Sanitize Data In $_GET 
 
Hehehe! I blame you MG because you didn't teach me properly!  

It was rotten script when I picked it up - and I just followed the rotten examples both with what I've got and the "Rotten Patches" that people like me post on the internet.

But now after spending more time reading the php Manual and not what some idiots write about how they fixed something - It's starting to make better sense.

 

But I still have a problem with the "New" password_hash( function - But I'll play some more and IF I can't fix it - It'll be over to you wizards to maybe shed some light on it.

 
 



 
mortSend private message  
Back to topPage bottom
Post new topic  Reply to topic  Page 1 of 1
 


Display posts from previous:    

HideWas this topic useful?

Link this topic
URL
BBCode
HTML




 
Permissions List
You cannot post new topics
You cannot reply to topics
You cannot edit your posts
You cannot delete your posts
You cannot vote in polls
You cannot attach files
You can download files
You cannot post calendar events


  

 

  cron