Security of User Add/Edit/Delete MySQL table

Clash Royale CLAN TAG#URR8PPP
.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty margin-bottom:0;
up vote
1
down vote
favorite
I have the following code which is working fine, however I am concerned around the security of this code and considering I am relatively new to php I was wondering whether the below code is secure?
I have a standard form page which submits the data to the table (testing.php). This is accompanied by another page that acts as an edit form for rows submitted forming part of the table (edit.php). These are as follows:
testing.php
<!DOCTYPE html>
<html>
<head>
<title>Database Table Submission</title>
</head>
<body>
<div>
<form method="POST" action="add.php">
<label>Product type:</label><input type="text" name="type">
<label>Description:</label><input type="text" name="description">
<label>Price:</label><input type="text" name="price">
<label>Expires:</label><input type="text" name="expiry">
<input type="submit" name="add">
</form>
</div>
<br>
<div>
<table border="1">
<thead>
<th>Type</th>
<th>Description</th>
<th>Price</th>
<th>Expires</th>
<th>Updates</th>
</thead>
<tbody>
<?php
include('includes/db.php');
$query=mysqli_query($mysqli,"select * from `products`");
while($row=mysqli_fetch_array($query))
?>
<tr>
<td><?php echo $row['type']; ?></td>
<td><?php echo $row['description']; ?></td>
<td><?php echo $row['price']; ?></td>
<td><?php echo $row['expiry']; ?></td>
<td>
<a href="edit.php?id=<?php echo $row['userid']; ?>">Edit</a>
<a href="delete.php?id=<?php echo $row['userid']; ?>">Delete</a>
</td>
</tr>
<?php
?>
</tbody>
</table>
</div>
</body>
</html>
<?php
include('includes/db.php');
$id=$_GET['id'];
$query=mysqli_query($mysqli,"select * from `products` where userid='$id'");
$row=mysqli_fetch_array($query);
?>
edit.php
<!DOCTYPE html>
<html>
<head>
<title>Edit</title>
</head>
<body>
<h2>Edit</h2>
<form method="POST" action="update.php?id=<?php echo $id; ?>">
<label>Type:</label><input type="text" value="<?php echo $row['type']; ?>" name="type">
<label>Description:</label><input type="text" value="<?php echo $row['description']; ?>" name="description">
<label>Price:</label><input type="text" value="<?php echo $row['price']; ?>" name="price">
<label>Expiry:</label><input type="text" value="<?php echo $row['expiry']; ?>" name="expiry">
<input type="submit" name="submit">
<a href="testing.php">Back</a>
</form>
</body>
</html>
3 php scripts do the work adding/updating and deleting information from the table (add.php, update.php and delete.php).
add.php
<?php
include('includes/db.php');
$type=$_POST['type'];
$description=$_POST['description'];
$price=$_POST['price'];
$expiry=$_POST['expiry'];
mysqli_query($mysqli,"insert into `products` (type, description, price, expiry) values ('$type','$description','$price','$expiry')");
header('location:testing.php');
?>
update.php
<?php
include('includes/db.php');
$id=$_GET['id'];
$type=$_POST['type'];
$description=$_POST['description'];
$price=$_POST['price'];
$expiry=$_POST['expiry'];
mysqli_query($mysqli,"update `products` set type='$type', description='$description', price='$price', expiry='$expiry' where userid='$id'");
header('location:testing.php');
?>
delete.php
<?php
$id=$_GET['id'];
include('includes/db.php');
mysqli_query($mysqli,"delete from `products` where userid='$id'");
header('location:testing.php');
?>
Any suggestions and guidance would be greatly appreciated.
Bonus: For some reason there is a significant delay in the table being updated? Not sure why this is?
php security mysqli
 |Â
show 3 more comments
up vote
1
down vote
favorite
I have the following code which is working fine, however I am concerned around the security of this code and considering I am relatively new to php I was wondering whether the below code is secure?
I have a standard form page which submits the data to the table (testing.php). This is accompanied by another page that acts as an edit form for rows submitted forming part of the table (edit.php). These are as follows:
testing.php
<!DOCTYPE html>
<html>
<head>
<title>Database Table Submission</title>
</head>
<body>
<div>
<form method="POST" action="add.php">
<label>Product type:</label><input type="text" name="type">
<label>Description:</label><input type="text" name="description">
<label>Price:</label><input type="text" name="price">
<label>Expires:</label><input type="text" name="expiry">
<input type="submit" name="add">
</form>
</div>
<br>
<div>
<table border="1">
<thead>
<th>Type</th>
<th>Description</th>
<th>Price</th>
<th>Expires</th>
<th>Updates</th>
</thead>
<tbody>
<?php
include('includes/db.php');
$query=mysqli_query($mysqli,"select * from `products`");
while($row=mysqli_fetch_array($query))
?>
<tr>
<td><?php echo $row['type']; ?></td>
<td><?php echo $row['description']; ?></td>
<td><?php echo $row['price']; ?></td>
<td><?php echo $row['expiry']; ?></td>
<td>
<a href="edit.php?id=<?php echo $row['userid']; ?>">Edit</a>
<a href="delete.php?id=<?php echo $row['userid']; ?>">Delete</a>
</td>
</tr>
<?php
?>
</tbody>
</table>
</div>
</body>
</html>
<?php
include('includes/db.php');
$id=$_GET['id'];
$query=mysqli_query($mysqli,"select * from `products` where userid='$id'");
$row=mysqli_fetch_array($query);
?>
edit.php
<!DOCTYPE html>
<html>
<head>
<title>Edit</title>
</head>
<body>
<h2>Edit</h2>
<form method="POST" action="update.php?id=<?php echo $id; ?>">
<label>Type:</label><input type="text" value="<?php echo $row['type']; ?>" name="type">
<label>Description:</label><input type="text" value="<?php echo $row['description']; ?>" name="description">
<label>Price:</label><input type="text" value="<?php echo $row['price']; ?>" name="price">
<label>Expiry:</label><input type="text" value="<?php echo $row['expiry']; ?>" name="expiry">
<input type="submit" name="submit">
<a href="testing.php">Back</a>
</form>
</body>
</html>
3 php scripts do the work adding/updating and deleting information from the table (add.php, update.php and delete.php).
add.php
<?php
include('includes/db.php');
$type=$_POST['type'];
$description=$_POST['description'];
$price=$_POST['price'];
$expiry=$_POST['expiry'];
mysqli_query($mysqli,"insert into `products` (type, description, price, expiry) values ('$type','$description','$price','$expiry')");
header('location:testing.php');
?>
update.php
<?php
include('includes/db.php');
$id=$_GET['id'];
$type=$_POST['type'];
$description=$_POST['description'];
$price=$_POST['price'];
$expiry=$_POST['expiry'];
mysqli_query($mysqli,"update `products` set type='$type', description='$description', price='$price', expiry='$expiry' where userid='$id'");
header('location:testing.php');
?>
delete.php
<?php
$id=$_GET['id'];
include('includes/db.php');
mysqli_query($mysqli,"delete from `products` where userid='$id'");
header('location:testing.php');
?>
Any suggestions and guidance would be greatly appreciated.
Bonus: For some reason there is a significant delay in the table being updated? Not sure why this is?
php security mysqli
1
On Stack Overflow it would have been closed as a duplicate for stackoverflow.com/q/60174/285587
â Your Common Sense
May 15 at 10:48
php.net/manual/en/security.database.sql-injection.php
â BCdotWEB
May 15 at 11:33
@YourCommonSense thanks for the link. In terms of sanitizing the above input, how would this be done? My assumption would be that this would have to be done in all files listed above for any instance where adjustment to the table is being performed?
â NickvR
May 15 at 13:19
@NickvR no need to write your code in the comments. write it in your editor and then run in your browser. As long as there are no variables in your queries, your code is safe.
â Your Common Sense
May 15 at 13:21
1
Please don't edit your code after receiving answers. If you want further advice on updated code, post a new question.
â Josiah
May 18 at 7:21
 |Â
show 3 more comments
up vote
1
down vote
favorite
up vote
1
down vote
favorite
I have the following code which is working fine, however I am concerned around the security of this code and considering I am relatively new to php I was wondering whether the below code is secure?
I have a standard form page which submits the data to the table (testing.php). This is accompanied by another page that acts as an edit form for rows submitted forming part of the table (edit.php). These are as follows:
testing.php
<!DOCTYPE html>
<html>
<head>
<title>Database Table Submission</title>
</head>
<body>
<div>
<form method="POST" action="add.php">
<label>Product type:</label><input type="text" name="type">
<label>Description:</label><input type="text" name="description">
<label>Price:</label><input type="text" name="price">
<label>Expires:</label><input type="text" name="expiry">
<input type="submit" name="add">
</form>
</div>
<br>
<div>
<table border="1">
<thead>
<th>Type</th>
<th>Description</th>
<th>Price</th>
<th>Expires</th>
<th>Updates</th>
</thead>
<tbody>
<?php
include('includes/db.php');
$query=mysqli_query($mysqli,"select * from `products`");
while($row=mysqli_fetch_array($query))
?>
<tr>
<td><?php echo $row['type']; ?></td>
<td><?php echo $row['description']; ?></td>
<td><?php echo $row['price']; ?></td>
<td><?php echo $row['expiry']; ?></td>
<td>
<a href="edit.php?id=<?php echo $row['userid']; ?>">Edit</a>
<a href="delete.php?id=<?php echo $row['userid']; ?>">Delete</a>
</td>
</tr>
<?php
?>
</tbody>
</table>
</div>
</body>
</html>
<?php
include('includes/db.php');
$id=$_GET['id'];
$query=mysqli_query($mysqli,"select * from `products` where userid='$id'");
$row=mysqli_fetch_array($query);
?>
edit.php
<!DOCTYPE html>
<html>
<head>
<title>Edit</title>
</head>
<body>
<h2>Edit</h2>
<form method="POST" action="update.php?id=<?php echo $id; ?>">
<label>Type:</label><input type="text" value="<?php echo $row['type']; ?>" name="type">
<label>Description:</label><input type="text" value="<?php echo $row['description']; ?>" name="description">
<label>Price:</label><input type="text" value="<?php echo $row['price']; ?>" name="price">
<label>Expiry:</label><input type="text" value="<?php echo $row['expiry']; ?>" name="expiry">
<input type="submit" name="submit">
<a href="testing.php">Back</a>
</form>
</body>
</html>
3 php scripts do the work adding/updating and deleting information from the table (add.php, update.php and delete.php).
add.php
<?php
include('includes/db.php');
$type=$_POST['type'];
$description=$_POST['description'];
$price=$_POST['price'];
$expiry=$_POST['expiry'];
mysqli_query($mysqli,"insert into `products` (type, description, price, expiry) values ('$type','$description','$price','$expiry')");
header('location:testing.php');
?>
update.php
<?php
include('includes/db.php');
$id=$_GET['id'];
$type=$_POST['type'];
$description=$_POST['description'];
$price=$_POST['price'];
$expiry=$_POST['expiry'];
mysqli_query($mysqli,"update `products` set type='$type', description='$description', price='$price', expiry='$expiry' where userid='$id'");
header('location:testing.php');
?>
delete.php
<?php
$id=$_GET['id'];
include('includes/db.php');
mysqli_query($mysqli,"delete from `products` where userid='$id'");
header('location:testing.php');
?>
Any suggestions and guidance would be greatly appreciated.
Bonus: For some reason there is a significant delay in the table being updated? Not sure why this is?
php security mysqli
I have the following code which is working fine, however I am concerned around the security of this code and considering I am relatively new to php I was wondering whether the below code is secure?
I have a standard form page which submits the data to the table (testing.php). This is accompanied by another page that acts as an edit form for rows submitted forming part of the table (edit.php). These are as follows:
testing.php
<!DOCTYPE html>
<html>
<head>
<title>Database Table Submission</title>
</head>
<body>
<div>
<form method="POST" action="add.php">
<label>Product type:</label><input type="text" name="type">
<label>Description:</label><input type="text" name="description">
<label>Price:</label><input type="text" name="price">
<label>Expires:</label><input type="text" name="expiry">
<input type="submit" name="add">
</form>
</div>
<br>
<div>
<table border="1">
<thead>
<th>Type</th>
<th>Description</th>
<th>Price</th>
<th>Expires</th>
<th>Updates</th>
</thead>
<tbody>
<?php
include('includes/db.php');
$query=mysqli_query($mysqli,"select * from `products`");
while($row=mysqli_fetch_array($query))
?>
<tr>
<td><?php echo $row['type']; ?></td>
<td><?php echo $row['description']; ?></td>
<td><?php echo $row['price']; ?></td>
<td><?php echo $row['expiry']; ?></td>
<td>
<a href="edit.php?id=<?php echo $row['userid']; ?>">Edit</a>
<a href="delete.php?id=<?php echo $row['userid']; ?>">Delete</a>
</td>
</tr>
<?php
?>
</tbody>
</table>
</div>
</body>
</html>
<?php
include('includes/db.php');
$id=$_GET['id'];
$query=mysqli_query($mysqli,"select * from `products` where userid='$id'");
$row=mysqli_fetch_array($query);
?>
edit.php
<!DOCTYPE html>
<html>
<head>
<title>Edit</title>
</head>
<body>
<h2>Edit</h2>
<form method="POST" action="update.php?id=<?php echo $id; ?>">
<label>Type:</label><input type="text" value="<?php echo $row['type']; ?>" name="type">
<label>Description:</label><input type="text" value="<?php echo $row['description']; ?>" name="description">
<label>Price:</label><input type="text" value="<?php echo $row['price']; ?>" name="price">
<label>Expiry:</label><input type="text" value="<?php echo $row['expiry']; ?>" name="expiry">
<input type="submit" name="submit">
<a href="testing.php">Back</a>
</form>
</body>
</html>
3 php scripts do the work adding/updating and deleting information from the table (add.php, update.php and delete.php).
add.php
<?php
include('includes/db.php');
$type=$_POST['type'];
$description=$_POST['description'];
$price=$_POST['price'];
$expiry=$_POST['expiry'];
mysqli_query($mysqli,"insert into `products` (type, description, price, expiry) values ('$type','$description','$price','$expiry')");
header('location:testing.php');
?>
update.php
<?php
include('includes/db.php');
$id=$_GET['id'];
$type=$_POST['type'];
$description=$_POST['description'];
$price=$_POST['price'];
$expiry=$_POST['expiry'];
mysqli_query($mysqli,"update `products` set type='$type', description='$description', price='$price', expiry='$expiry' where userid='$id'");
header('location:testing.php');
?>
delete.php
<?php
$id=$_GET['id'];
include('includes/db.php');
mysqli_query($mysqli,"delete from `products` where userid='$id'");
header('location:testing.php');
?>
Any suggestions and guidance would be greatly appreciated.
Bonus: For some reason there is a significant delay in the table being updated? Not sure why this is?
php security mysqli
edited May 18 at 7:29
Mathias Ettinger
21.8k32875
21.8k32875
asked May 15 at 9:54
NickvR
143
143
1
On Stack Overflow it would have been closed as a duplicate for stackoverflow.com/q/60174/285587
â Your Common Sense
May 15 at 10:48
php.net/manual/en/security.database.sql-injection.php
â BCdotWEB
May 15 at 11:33
@YourCommonSense thanks for the link. In terms of sanitizing the above input, how would this be done? My assumption would be that this would have to be done in all files listed above for any instance where adjustment to the table is being performed?
â NickvR
May 15 at 13:19
@NickvR no need to write your code in the comments. write it in your editor and then run in your browser. As long as there are no variables in your queries, your code is safe.
â Your Common Sense
May 15 at 13:21
1
Please don't edit your code after receiving answers. If you want further advice on updated code, post a new question.
â Josiah
May 18 at 7:21
 |Â
show 3 more comments
1
On Stack Overflow it would have been closed as a duplicate for stackoverflow.com/q/60174/285587
â Your Common Sense
May 15 at 10:48
php.net/manual/en/security.database.sql-injection.php
â BCdotWEB
May 15 at 11:33
@YourCommonSense thanks for the link. In terms of sanitizing the above input, how would this be done? My assumption would be that this would have to be done in all files listed above for any instance where adjustment to the table is being performed?
â NickvR
May 15 at 13:19
@NickvR no need to write your code in the comments. write it in your editor and then run in your browser. As long as there are no variables in your queries, your code is safe.
â Your Common Sense
May 15 at 13:21
1
Please don't edit your code after receiving answers. If you want further advice on updated code, post a new question.
â Josiah
May 18 at 7:21
1
1
On Stack Overflow it would have been closed as a duplicate for stackoverflow.com/q/60174/285587
â Your Common Sense
May 15 at 10:48
On Stack Overflow it would have been closed as a duplicate for stackoverflow.com/q/60174/285587
â Your Common Sense
May 15 at 10:48
php.net/manual/en/security.database.sql-injection.php
â BCdotWEB
May 15 at 11:33
php.net/manual/en/security.database.sql-injection.php
â BCdotWEB
May 15 at 11:33
@YourCommonSense thanks for the link. In terms of sanitizing the above input, how would this be done? My assumption would be that this would have to be done in all files listed above for any instance where adjustment to the table is being performed?
â NickvR
May 15 at 13:19
@YourCommonSense thanks for the link. In terms of sanitizing the above input, how would this be done? My assumption would be that this would have to be done in all files listed above for any instance where adjustment to the table is being performed?
â NickvR
May 15 at 13:19
@NickvR no need to write your code in the comments. write it in your editor and then run in your browser. As long as there are no variables in your queries, your code is safe.
â Your Common Sense
May 15 at 13:21
@NickvR no need to write your code in the comments. write it in your editor and then run in your browser. As long as there are no variables in your queries, your code is safe.
â Your Common Sense
May 15 at 13:21
1
1
Please don't edit your code after receiving answers. If you want further advice on updated code, post a new question.
â Josiah
May 18 at 7:21
Please don't edit your code after receiving answers. If you want further advice on updated code, post a new question.
â Josiah
May 18 at 7:21
 |Â
show 3 more comments
1 Answer
1
active
oldest
votes
up vote
0
down vote
accepted
Your code is vulnerable to both SQL injection and HTML injection. So on Stack Overflow, this question would be a duplicate of both How can I prevent SQL injection in PHP? and How to prevent XSS with HTML/PHP?
Code like
<td><?php echo $row['type']; ?></td>
would become something like
<td><?php echo htmlspecialchars($row['type'], ENT_QUOTES, 'UTF-8'); ?></td>
and code like
$id=$_GET['id'];
$query=mysqli_query($mysqli,"select * from `products` where userid='$id'");
could become something like
$id = mysqli_real_escape_string($mysqli, $_GET['id']);
$query = mysqli_query($mysqli, "select * from `products` where userid='$id'");
And yes, as you speculated in a comment, you would have to do these in each file unless you rework things so as to make them happen automatically. E.g. something like
<td><?php safe_echo($row['type']); ?></td>
where
function safe_echo($unknown_output)
$safe_output = htmlspecialchars($unknown_output, ENT_QUOTES, 'UTF-8');
echo $safe_output;
For the SQL injection, a more common way to approach this is parameterized queries.
$statement = mysqli_prepare($mysqli, "SELECT * FROM `products` WHERE userid=?");
mysqli_stmt_bind_param($statement, 's', $_GET['id'];
mysqli_stmt_execute($statement);
mysqli_stmt_bind_result($statement, $type, $description, $price, $expiry);
And you would use the results like
while (mysqli_stmt_fetch($statement))
echo <<<EOHTML
<td><?php echo $type; ?></td>
<td><?php echo $description; ?></td>
<td><?php echo $price; ?></td>
<td><?php echo $expiry; ?></td>
EOHTML;
mysqli_stmt_close($statement);
Parameterized queries don't require separate escaping. The binding process will handle that automatically.
In both cases, you still have to change every file, but you don't necessarily have to write out the solution everywhere.
thanks for the notes @mdfst13, I'll do a bit of a rework on this and see if I can improve these vulnerabilities.
â NickvR
May 16 at 6:43
"could become something like" is no way a solid explanation, that makes it most likely to be misunderstood. There are enough PHP users who believe in the "escape string is intended to protect from SQL injection" nonsense already, no need to add another to their number.
â Your Common Sense
May 16 at 7:06
The update includes prepared statements? @YourCommonSense
â NickvR
May 18 at 6:35
add a comment |Â
1 Answer
1
active
oldest
votes
1 Answer
1
active
oldest
votes
active
oldest
votes
active
oldest
votes
up vote
0
down vote
accepted
Your code is vulnerable to both SQL injection and HTML injection. So on Stack Overflow, this question would be a duplicate of both How can I prevent SQL injection in PHP? and How to prevent XSS with HTML/PHP?
Code like
<td><?php echo $row['type']; ?></td>
would become something like
<td><?php echo htmlspecialchars($row['type'], ENT_QUOTES, 'UTF-8'); ?></td>
and code like
$id=$_GET['id'];
$query=mysqli_query($mysqli,"select * from `products` where userid='$id'");
could become something like
$id = mysqli_real_escape_string($mysqli, $_GET['id']);
$query = mysqli_query($mysqli, "select * from `products` where userid='$id'");
And yes, as you speculated in a comment, you would have to do these in each file unless you rework things so as to make them happen automatically. E.g. something like
<td><?php safe_echo($row['type']); ?></td>
where
function safe_echo($unknown_output)
$safe_output = htmlspecialchars($unknown_output, ENT_QUOTES, 'UTF-8');
echo $safe_output;
For the SQL injection, a more common way to approach this is parameterized queries.
$statement = mysqli_prepare($mysqli, "SELECT * FROM `products` WHERE userid=?");
mysqli_stmt_bind_param($statement, 's', $_GET['id'];
mysqli_stmt_execute($statement);
mysqli_stmt_bind_result($statement, $type, $description, $price, $expiry);
And you would use the results like
while (mysqli_stmt_fetch($statement))
echo <<<EOHTML
<td><?php echo $type; ?></td>
<td><?php echo $description; ?></td>
<td><?php echo $price; ?></td>
<td><?php echo $expiry; ?></td>
EOHTML;
mysqli_stmt_close($statement);
Parameterized queries don't require separate escaping. The binding process will handle that automatically.
In both cases, you still have to change every file, but you don't necessarily have to write out the solution everywhere.
thanks for the notes @mdfst13, I'll do a bit of a rework on this and see if I can improve these vulnerabilities.
â NickvR
May 16 at 6:43
"could become something like" is no way a solid explanation, that makes it most likely to be misunderstood. There are enough PHP users who believe in the "escape string is intended to protect from SQL injection" nonsense already, no need to add another to their number.
â Your Common Sense
May 16 at 7:06
The update includes prepared statements? @YourCommonSense
â NickvR
May 18 at 6:35
add a comment |Â
up vote
0
down vote
accepted
Your code is vulnerable to both SQL injection and HTML injection. So on Stack Overflow, this question would be a duplicate of both How can I prevent SQL injection in PHP? and How to prevent XSS with HTML/PHP?
Code like
<td><?php echo $row['type']; ?></td>
would become something like
<td><?php echo htmlspecialchars($row['type'], ENT_QUOTES, 'UTF-8'); ?></td>
and code like
$id=$_GET['id'];
$query=mysqli_query($mysqli,"select * from `products` where userid='$id'");
could become something like
$id = mysqli_real_escape_string($mysqli, $_GET['id']);
$query = mysqli_query($mysqli, "select * from `products` where userid='$id'");
And yes, as you speculated in a comment, you would have to do these in each file unless you rework things so as to make them happen automatically. E.g. something like
<td><?php safe_echo($row['type']); ?></td>
where
function safe_echo($unknown_output)
$safe_output = htmlspecialchars($unknown_output, ENT_QUOTES, 'UTF-8');
echo $safe_output;
For the SQL injection, a more common way to approach this is parameterized queries.
$statement = mysqli_prepare($mysqli, "SELECT * FROM `products` WHERE userid=?");
mysqli_stmt_bind_param($statement, 's', $_GET['id'];
mysqli_stmt_execute($statement);
mysqli_stmt_bind_result($statement, $type, $description, $price, $expiry);
And you would use the results like
while (mysqli_stmt_fetch($statement))
echo <<<EOHTML
<td><?php echo $type; ?></td>
<td><?php echo $description; ?></td>
<td><?php echo $price; ?></td>
<td><?php echo $expiry; ?></td>
EOHTML;
mysqli_stmt_close($statement);
Parameterized queries don't require separate escaping. The binding process will handle that automatically.
In both cases, you still have to change every file, but you don't necessarily have to write out the solution everywhere.
thanks for the notes @mdfst13, I'll do a bit of a rework on this and see if I can improve these vulnerabilities.
â NickvR
May 16 at 6:43
"could become something like" is no way a solid explanation, that makes it most likely to be misunderstood. There are enough PHP users who believe in the "escape string is intended to protect from SQL injection" nonsense already, no need to add another to their number.
â Your Common Sense
May 16 at 7:06
The update includes prepared statements? @YourCommonSense
â NickvR
May 18 at 6:35
add a comment |Â
up vote
0
down vote
accepted
up vote
0
down vote
accepted
Your code is vulnerable to both SQL injection and HTML injection. So on Stack Overflow, this question would be a duplicate of both How can I prevent SQL injection in PHP? and How to prevent XSS with HTML/PHP?
Code like
<td><?php echo $row['type']; ?></td>
would become something like
<td><?php echo htmlspecialchars($row['type'], ENT_QUOTES, 'UTF-8'); ?></td>
and code like
$id=$_GET['id'];
$query=mysqli_query($mysqli,"select * from `products` where userid='$id'");
could become something like
$id = mysqli_real_escape_string($mysqli, $_GET['id']);
$query = mysqli_query($mysqli, "select * from `products` where userid='$id'");
And yes, as you speculated in a comment, you would have to do these in each file unless you rework things so as to make them happen automatically. E.g. something like
<td><?php safe_echo($row['type']); ?></td>
where
function safe_echo($unknown_output)
$safe_output = htmlspecialchars($unknown_output, ENT_QUOTES, 'UTF-8');
echo $safe_output;
For the SQL injection, a more common way to approach this is parameterized queries.
$statement = mysqli_prepare($mysqli, "SELECT * FROM `products` WHERE userid=?");
mysqli_stmt_bind_param($statement, 's', $_GET['id'];
mysqli_stmt_execute($statement);
mysqli_stmt_bind_result($statement, $type, $description, $price, $expiry);
And you would use the results like
while (mysqli_stmt_fetch($statement))
echo <<<EOHTML
<td><?php echo $type; ?></td>
<td><?php echo $description; ?></td>
<td><?php echo $price; ?></td>
<td><?php echo $expiry; ?></td>
EOHTML;
mysqli_stmt_close($statement);
Parameterized queries don't require separate escaping. The binding process will handle that automatically.
In both cases, you still have to change every file, but you don't necessarily have to write out the solution everywhere.
Your code is vulnerable to both SQL injection and HTML injection. So on Stack Overflow, this question would be a duplicate of both How can I prevent SQL injection in PHP? and How to prevent XSS with HTML/PHP?
Code like
<td><?php echo $row['type']; ?></td>
would become something like
<td><?php echo htmlspecialchars($row['type'], ENT_QUOTES, 'UTF-8'); ?></td>
and code like
$id=$_GET['id'];
$query=mysqli_query($mysqli,"select * from `products` where userid='$id'");
could become something like
$id = mysqli_real_escape_string($mysqli, $_GET['id']);
$query = mysqli_query($mysqli, "select * from `products` where userid='$id'");
And yes, as you speculated in a comment, you would have to do these in each file unless you rework things so as to make them happen automatically. E.g. something like
<td><?php safe_echo($row['type']); ?></td>
where
function safe_echo($unknown_output)
$safe_output = htmlspecialchars($unknown_output, ENT_QUOTES, 'UTF-8');
echo $safe_output;
For the SQL injection, a more common way to approach this is parameterized queries.
$statement = mysqli_prepare($mysqli, "SELECT * FROM `products` WHERE userid=?");
mysqli_stmt_bind_param($statement, 's', $_GET['id'];
mysqli_stmt_execute($statement);
mysqli_stmt_bind_result($statement, $type, $description, $price, $expiry);
And you would use the results like
while (mysqli_stmt_fetch($statement))
echo <<<EOHTML
<td><?php echo $type; ?></td>
<td><?php echo $description; ?></td>
<td><?php echo $price; ?></td>
<td><?php echo $expiry; ?></td>
EOHTML;
mysqli_stmt_close($statement);
Parameterized queries don't require separate escaping. The binding process will handle that automatically.
In both cases, you still have to change every file, but you don't necessarily have to write out the solution everywhere.
answered May 15 at 22:10
mdfst13
16.8k42055
16.8k42055
thanks for the notes @mdfst13, I'll do a bit of a rework on this and see if I can improve these vulnerabilities.
â NickvR
May 16 at 6:43
"could become something like" is no way a solid explanation, that makes it most likely to be misunderstood. There are enough PHP users who believe in the "escape string is intended to protect from SQL injection" nonsense already, no need to add another to their number.
â Your Common Sense
May 16 at 7:06
The update includes prepared statements? @YourCommonSense
â NickvR
May 18 at 6:35
add a comment |Â
thanks for the notes @mdfst13, I'll do a bit of a rework on this and see if I can improve these vulnerabilities.
â NickvR
May 16 at 6:43
"could become something like" is no way a solid explanation, that makes it most likely to be misunderstood. There are enough PHP users who believe in the "escape string is intended to protect from SQL injection" nonsense already, no need to add another to their number.
â Your Common Sense
May 16 at 7:06
The update includes prepared statements? @YourCommonSense
â NickvR
May 18 at 6:35
thanks for the notes @mdfst13, I'll do a bit of a rework on this and see if I can improve these vulnerabilities.
â NickvR
May 16 at 6:43
thanks for the notes @mdfst13, I'll do a bit of a rework on this and see if I can improve these vulnerabilities.
â NickvR
May 16 at 6:43
"could become something like" is no way a solid explanation, that makes it most likely to be misunderstood. There are enough PHP users who believe in the "escape string is intended to protect from SQL injection" nonsense already, no need to add another to their number.
â Your Common Sense
May 16 at 7:06
"could become something like" is no way a solid explanation, that makes it most likely to be misunderstood. There are enough PHP users who believe in the "escape string is intended to protect from SQL injection" nonsense already, no need to add another to their number.
â Your Common Sense
May 16 at 7:06
The update includes prepared statements? @YourCommonSense
â NickvR
May 18 at 6:35
The update includes prepared statements? @YourCommonSense
â NickvR
May 18 at 6:35
add a comment |Â
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f194442%2fsecurity-of-user-add-edit-delete-mysql-table%23new-answer', 'question_page');
);
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
1
On Stack Overflow it would have been closed as a duplicate for stackoverflow.com/q/60174/285587
â Your Common Sense
May 15 at 10:48
php.net/manual/en/security.database.sql-injection.php
â BCdotWEB
May 15 at 11:33
@YourCommonSense thanks for the link. In terms of sanitizing the above input, how would this be done? My assumption would be that this would have to be done in all files listed above for any instance where adjustment to the table is being performed?
â NickvR
May 15 at 13:19
@NickvR no need to write your code in the comments. write it in your editor and then run in your browser. As long as there are no variables in your queries, your code is safe.
â Your Common Sense
May 15 at 13:21
1
Please don't edit your code after receiving answers. If you want further advice on updated code, post a new question.
â Josiah
May 18 at 7:21