How to Secure My Code

Question

I'm writing a simple web-based site for a company to display products on their site. It needs to be fairly easy to maintain. I'm not able to use a database. I'm using a multidimensional array to store the product information and retrieving it with a product key.

My main concern is security. I have a very limited amount of time I can spend on this - so, I don't have the bandwidth to build anything more serious. If you see anything that looks obviously bad, please let me know how I can patch it.

Here is an example URL with the product key: http://example.com/products.php?productKey=widget

Here is the code that gets the product key, verifies it's valid, and retrieves the product information:

// obtain merchandise variables include 'merch.vars.php'; // assign a default value $productKey = 'placeholder'; // check to see if a value was passed if (isset($_GET["productKey"])) { // create array of product keys $productArrayKeys = array_keys($product); // check if value passed to page exists in product key array if (in_array($_GET["productKey"], $productArrayKeys)) { // value exists - assign to $productKey $productKey = $_GET["productKey"]; } }

Here is an example of the product multidimensional array:

$product = array( "placeholder" => array( item_title => "Placeholder Title", item_image_url => "placeholder.png", item_price => "0.00", item_description => "Placeholder Description", item_quantity => 1, product_icons => false ), "widget" => array( item_title => "Product Title", item_image_url => "widget.png", item_price => "15.00", item_description => "Product Description", item_quantity => 1, item_category => array( small => "Small", medium => "Medium", large => "Large", Xlarge => "XLarge" ), product_icons => true ) );

What type of attacks do you need to defend against? For example, is it a problem if an attacker guesses a product key for a product that the site didn't tell them about? Nothing in the code you posted actually does anything with lasting effects so it's hard to guess what sort of effects are acceptable vs. undesired. — Wyzard
– Wyzard, Commented Mar 29, 2012 at 5:28
Without a database, and likely without an authentication system, I don't see many possible breaches...This part you posted looks "secure", though. Sidenote: how many products are there? Are you sure this approach will be "fairly easy to maintain" ? — Damien Pirsy
– Damien Pirsy, Commented Mar 29, 2012 at 5:31
you use php arrays... and SQL injections and data corruption is not possible as long your php file that has the arrays is still alive — Netorica
– Netorica, Commented Mar 29, 2012 at 5:31

David Stockton · Accepted Answer · 2012-03-29 05:36:56Z

It looks like you've got a decent way to validate that someone cannot pass in something that doesn't exist in your product array. That said, your description/comments about it retrieving the information from the products array is not quite valid. After doing the checks, you'd need to have something like

$chosenProduct = $product[$productKey];

in order to actually get the product information.

One more point of order, in your $product array, it really should have all of your keys quoted like so:

$product = array( "placeholder" => array( "item_title" => "Placeholder Title", "item_image_url" => "placeholder.png", "item_price" => "0.00", "item_description" => "Placeholder Description", "item_quantity" => 1, "product_icons" => false ), "widget" => array( "item_title" => "Product Title", "item_image_url" => "widget.png", "item_price" => "15.00", "item_description" => "Product Description", "item_quantity" => 1, "item_category" => array( "small" => "Small", "medium" => "Medium", "large" => "Large", "Xlarge" => "XLarge" ), "product_icons" => true ) );

Without quoting, PHP will make an assumption that you're using constants. It will try to look up the value of the constants and assuming you don't have any constants that match any of those names, will throw a notice and tell you it's assuming you meant to use a string. Quoting will make it perform better and will not conflict with any constants that may have been defined with any of those keys.

Nice catch David, although I've always preferred single quotes.
I prefer single as well, but matched with the style of the OP since all the value parts used double quotes.

stewe · Accepted Answer · 2012-03-29 05:40:19Z

I think you want something like this:

if (isset($_GET["productKey"]) && isset($product[$_GET['productKey']])) { $productKey = $_GET["productKey"]; print_r($product[$productKey]); }else{ echo 'product does not exits / productKey not set'; }

Collectives™ on Stack Overflow

How to Secure My Code

2 Answers 2

2 Comments

Comments

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

2 Comments

Comments

Related