patternphpMinor
Increase view counter with each page view
Viewed 0 times
eachwithviewincreasepagecounter
Problem
My question is similar to before, but now the code has changed completely. I would like to understand if this code is vulnerable to mysql injection.
0) {
// Connect to the database
include_once("../configuration.php");
$cg = new JConfig;
$con = mysqli_connect($cg->host,$cg->user,$cg->password,$cg->db);
if (mysqli_connect_errno()) {
die('n/a');
}
// Add new hit: Update the `times_viewed` field corresponding to specific video id
$query = "UPDATE " . $cg->dbprefix . "hdflv_upload
SET `times_viewed` = `times_viewed` + 1
WHERE `id` LIKE " . $id . ";";
if (mysqli_query($con, $query) === true) {
// Get the updated `times_viewed` of video id
$query = "SELECT `times_viewed`
FROM " . $cg->dbprefix . "hdflv_upload
WHERE `id` LIKE " . $id . ";";
$result = mysqli_fetch_assoc(mysqli_query($con, $query));
}
// close the connection to the database
mysqli_close($con);
$addone = $result['times_viewed'];
echo $addone;
}
?>Solution
No, this code is not vulnerable to SQL injection. The use of
It is very weird that you use the
Note that in this version, you have fixed a race condition that existed in the previous version. In the previous version, it is possible to under-count if two pages simultaneously try to update the count, and the sequence ends up being
intval() and if (… && $id > 0) guarantee that $id is a positive integer, so the UPDATE and SELECT queries cannot be malicious.It is very weird that you use the
LIKE operator, though. Is the id column not an INTEGER? I would expect that that works by converting every id into a string, then performing pattern matching. That would be slow because- Converting every
idin thehdflv_uploadtable would be extra work.
- The database cannot locate the row with the right
idusing an index; it has to examine everyid.
- Pattern matching is more complicated than a simple integer equality check.
Note that in this version, you have fixed a race condition that existed in the previous version. In the previous version, it is possible to under-count if two pages simultaneously try to update the count, and the sequence ends up being
SELECT, SELECT, UPDATE, UPDATE. This code is better because UPDATE hdflv_upload SET times_viewed = times_viewed + 1 WHERE … should be immune to race conditions.Context
StackExchange Code Review Q#115223, answer score: 3
Revisions (0)
No revisions yet.