Page 1 of 1
Could there be any security issues with this code ?
Posted: Thu Sep 04, 2008 3:25 pm
by guischarf
Hello,
Could this --using dynamic variables to fill up a form-- have security issues ? This is an update form. A user logins to his account and gets back his data in "<form>", where he can update it.
I am checking all the data before it goes back to the BD.
Code: Select all
<?php
$qry = "SELECT name, lastname [, ..., ...] FROM table WHERE id = 'someid' ";
$res = mysql_query($qry);
while ($row = mysql_fetch_assoc($res)):
$records[] = $row
endwhile;
foreach ($records as $record):
foreach($record as $key => $value)
$thisvar = $key;
$$thisvar = $value;
endforeach;
endforeach;
?>
<form>
<input name="name" value="<?php echo $name ?>">
<input name="lastname" value="<?php echo $lastname?>">
[...]
</form>
Tnx.
Re: Could there be any security issues with this code ?
Posted: Thu Sep 04, 2008 7:24 pm
by andyhoneycutt
I would say that potentially any piece of code brings with it some possible security risk. As long as you are type-checking the values that the user submits and properly escaping your data input, you should be about as safe as anyone else out there. sprintf is a great function for forcing values to adhere to their types, and any time you plan on inserting data that comes from any user at any point on the path to database entry it is always good practice to escape the data before inputting it.
That said, I see no glaring errors in the piece of code you provide as long as the "someid" variable you are using to run your select statement is properly escaped and cleaned.
-Andy
Re: Could there be any security issues with this code ?
Posted: Fri Sep 05, 2008 2:15 am
by Mordred
andyhoneycutt wrote: and any time you plan on inserting data that comes from any user at any point on the path to database entry it is always good practice to escape the data before inputting it.
No. No conditionals. Escape
always all data.
http://www.logris.org/security/escaping ... ty-measure
guischarf, there are possible XSS issues, and related problems with register_globals=on. Also, how secure is a single piece of code may change with the context - who uses this, in what way, etc. Especially seeing that this is not your actual code, but an abridged version of it, we can't really say anything of the piece itself. Maybe this is safe, but the code updating the user isn't. We can't tell.
Re: Could there be any security issues with this code ?
Posted: Fri Sep 05, 2008 2:55 am
by Christopher
I agree, both $name and $lastname are potential sources of attacks it the code above.
Re: Could there be any security issues with this code ?
Posted: Mon Sep 22, 2008 2:27 pm
by allicient
Mordred's comment about context is especially relevant here: if 'someid' is under control of the client, and is not explicitly bound to an active session then you've just allowed anyone and their dog to enumerate your entire database table (not a great move). But I'm guessing 'someid' is protected and tied to the user's valid session, right?
You've probably not implemented things like what you've shown (with the php tags in the form), but an additional suggestion: always seperate your code from your data... i.e. mixing lots of <?php blah blah ?> into the middle of an HTML page is going to make it practically unreadably and prone to errors. My advice (and I know there's a lot of people who disagree on this) is to have the HTTP GET or POST request to whatever.php and then have the php code output HTML through echo or whatever (you can load big chunks of HTML from a file). Keeps it simple. I've found Smarty (templating system) to be very useful in the past for splitting code and data (plus you can do some really nifty stuff with it), although I've not had the time to assess the security of the Smarty libraries to date.