Discussions of secure PHP coding. Security in software is important, so don't be afraid to ask. And when answering: be anal. Nitpick. No security vulnerability is too small.
$pid = intval($_GET['pid']);
// or
$pid = preg_replace('/[^0-9]/', '', $_GET['pid']);
I would also recommend rearranging things a little to promote Defense in Depth. For example, remove slashes from the entire Request (if magic_quotes is set) early in the script to make sure nothing slips through. Likewise, I would do the database escaping right before the query code so you can be sure it is getting done. If code is scattered, it is possible to make unintended changes later.
I have to disagree with using intval because I am in the mindset that a variable shouldn't be forced to be something it isn't. I believe this mindset could potentially prevent unintended side effects. I would encourage the use of preg_match in combination with trim to validate that it is an integer.
With the example below you can also check for min and max len so it's win win.
Thanks Astions, i'll have a read up on trim and preg_match. I assume that the {1,10} part of it is the min and max length settings yeah? What does the two # and $ sign do in the preg_match?
This is what I came up with
(Had to edit because I missed out part of the mysql_real_escape. I did not work when I tested it without the filtering before it. So I fixed it and it now works)
@frozen: Looks good, you're getting the hang of it.
@arborint: Yes, I think there is a fundamental difference between modifying something to make it fit and verifying that it will fit. I'm sure there are a lot of for and against arguments for either, and a lot of hypothetical situations as well. At the end of the day though it just makes more sense to throw an error if the data is of the wrong type. It would certainly indicate a higher level problem, to me at least.
From a "lazy safe" perspective. Wouldn't just escaping the input be enough? I mean if somone enters a non int to compare to the ID(int) column mysql would just return 0 rows?
astions wrote:...
@arborint: Yes, I think there is a fundamental difference between modifying something to make it fit and verifying that it will fit. I'm sure there are a lot of for and against arguments for either, and a lot of hypothetical situations as well. At the end of the day though it just makes more sense to throw an error if the data is of the wrong type. It would certainly indicate a higher level problem, to me at least.
For integers in particular it makes not difference whatsoever I think. You can easily pass any id anyhow....sow it wouldn't make a difference.
I'm going to implement paging and a where clause to pick products in a category.
I have been reading around about security for these two bits and I would just like to see if I'm on the right track.
For the paging I'm going to filter for only numbers. Use a calculation in the script, so that if someone did get anything else through. It should bring up an error. And am I right in thinking that I should quote the variable in the LIMIT of the SQL script but not use mysql real esscape strings?
For the categories I was thinking of doing an SQL query to get all the categories and automaticly create a variable with the name of the category and with the value of the ID number. And then with the GET I would filter for numbers and only take 3 digits. And if that number matches one of the variable values of the categories, then the script will then take that variable and use that in the where clause with real esscape strings.
How do they sound? I am right in thinking that I can automaticly make variables from an sql command and match it to a GET imput ain't I?
I'm also planning to have reviews for products once the site is up and running. I was thinking of just using a mySQL account that can only write to this table and not read or update etc. This is a good idea yes? I still need to do some research in how add security to this because I don't want to block everything that isn't a number or character, so if anyone has any links they can give me that can help me with this I would be grateful.