Security of User Add/Edit/Delete MySQL table

The name of the pictureThe name of the pictureThe name of the pictureClash 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?







share|improve this question

















  • 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
















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?







share|improve this question

















  • 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












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?







share|improve this question













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?









share|improve this question












share|improve this question




share|improve this question








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












  • 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










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.






share|improve this answer





















  • 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











Your Answer




StackExchange.ifUsing("editor", function ()
return StackExchange.using("mathjaxEditing", function ()
StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix)
StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
);
);
, "mathjax-editing");

StackExchange.ifUsing("editor", function ()
StackExchange.using("externalEditor", function ()
StackExchange.using("snippets", function ()
StackExchange.snippets.init();
);
);
, "code-snippets");

StackExchange.ready(function()
var channelOptions =
tags: "".split(" "),
id: "196"
;
initTagRenderer("".split(" "), "".split(" "), channelOptions);

StackExchange.using("externalEditor", function()
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled)
StackExchange.using("snippets", function()
createEditor();
);

else
createEditor();

);

function createEditor()
StackExchange.prepareEditor(
heartbeatType: 'answer',
convertImagesToLinks: false,
noModals: false,
showLowRepImageUploadWarning: true,
reputationToPostImages: null,
bindNavPrevention: true,
postfix: "",
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
);



);








 

draft saved


draft discarded


















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






























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.






share|improve this answer





















  • 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















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.






share|improve this answer





















  • 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













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.






share|improve this answer













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.







share|improve this answer













share|improve this answer



share|improve this answer











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

















  • 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













 

draft saved


draft discarded


























 


draft saved


draft discarded














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













































































Popular posts from this blog

Python Lists

Aion

JavaScript Array Iteration Methods