Im building a cms and have written the first part that just displays the contnet from a database into a template.
It all works, however i need to know if i have checked things properly / security in my code. Have i written effeciently...etc.
Some feed back would be good.
Hi,
Your code has at least one big security flaw. You are using GET variables in your sql query without doing any checking.
Please think of what would happen if I would request 'http://yourdomain.com/?page=0; DROP TABLE c'
This is a technique called 'SQL injection'.
You should check all your variables, no matter if they are $GET, $POST, $_COOKIE, etc. At the very least use mysql_real_escape_string or mysql_escape_string to escape al the special characters.
Also check http://www.php.net/manual/en/security.database.sql-injection.php for more information on this subject.
Ok thanks frikikip this is a really big flaw. Other than that are there any more checks that i can be doing?
When I look at your code I notice that you include your dbconnect.inc.php quite often. You probably connect to your database with each include. This isn't really necessary.
You should only connect once every page request (if you are not using multiple databases that is).
Other than that I don't really see any flaws.
i connect to my database at the top of the page, however, i cannot use this connection within my functions?
If you look at my changes in regards to the $_GET, i have tried to make it safer by using settype() and sprintf(), is this valid.
Hi,
I thought you reconnected to your database with each include. The way you are doing it now is the correct way.
You're query is now safe as far as I can see.
While im here im having problems with my image function that brings up all the image paths in the database depending on whether they have been specified to be..ie a field called front - if it has a 1 then it will be included if 0 not.
I want to count the number of rows with 1 and if there arent any display a message saying "no images" otherwise it will display the paths.
this is my effort that doesnt work:
function images() {
include("dbconnect.inc.php");
$sql2 = "select * from images where front =1";
$result3 = mysql_query($sql2,$conn);
$image = $row3['path'];
$front = $row3['front'];
echo "<table class='block' align='center' width='100%' cellpadding=0 cellspacing=0>";
echo "<tr><td class='heading'>Gallery</td></tr>";
if($front <0) {
echo "<tr><td>no images</td></tr>";
}else {
while($row3 = mysql_fetch_assoc($result3))
{
echo "<tr><td><img src='images/$image'></td></tr>";
}
}
echo "</table>";
}
Hi tbobker
If you want to count the number of rows your query has returned use mysql_num_rows
so try this:
<?php
function images() {
// If you connect to your database in dbconnect.inc.php,
// you will reconnect everytime you include dbconnect.inc.php.
// Creating to your database takes resources (cpu time, memory, etc)
// So its best to create the connection only once
// include("dbconnect.inc.php");
// The best way is to include dbconnect.inc.php once at the top
// of your script and make your connection global
global $conn;
$sql2 = "select * from images where front =1";
$result3 = mysql_query($sql2,$conn);
echo "<table class='block' align='center' width='100%' cellpadding=0 cellspacing=0>";
echo "<tr><td class='heading'>Gallery</td></tr>";
if(mysql_num_rows($result3) > 0) {
echo "<tr><td>no images</td></tr>";
}else {
while($row3 = mysql_fetch_assoc($result3))
{
echo "<tr><td><img src='images/$image'></td></tr>";
}
}
echo "</table>";
}
?>