patternphpMinor
Adding an item to database + JSON
Viewed 0 times
itemdatabasejsonadding
Problem
Is this code good? Or is it the noobiest PHP you've ever seen?
"ERROR: No Data Recieved"));
} else if(!isset($_POST["name"]) || $_POST["name"] == "") {
send(array("msg" => "ERROR: Name field cannot be left blank"));
} else if(!isset($_POST["class"])) {
send(array("msg" => "ERROR: Class field cannot be left blank"));
}
//Set $_POST Data
$name = $_POST["name"];
$cid = $_POST["class"];
//Check if PHP is connected to database
if(!$sql) {
send(array("msg" => "ERROR: Cannot Connect to Database"));
}
//Check for Existing Handout
$query = "SELECT id FROM Handouts WHERE name='$name' AND cid=$cid";
$result = mysqli_query($sql, $query);
if(!$result) {
send(array("msg" => "ERROR: Cannot Connect to Database"));
} else if(mysqli_num_rows($result) > 0) {
send(array("msg" => "ERROR: Handout already exists"));
}
//Add Handout to DB
$query = "INSERT INTO Handouts (name, cid) VALUES ('$name', $cid)";
$result = mysqli_query($sql, $query);
if(!$result) {
send(array("msg" => "ERROR: Cannot Connect to Database"));
}
//Get Handout ID
$query = "SELECT id FROM Handouts WHERE name='$name' AND cid=$cid";
$result = mysqli_query($sql, $query);
if(!$result) {
send(array("msg" => "ERROR: Cannot Connect to Database"));
}
$hid = mysqli_fetch_assoc($result)["id"];
//Get JSON from file
$json = jread("../data.json");
//Add handout to JSON
$json[$hid] = array();
//Write JSON to file
jwrite("../data.json", $json);
//Close Connection to DB
mysqli_close($sql);
//Return to Web Form
send(array("msg" => "SUCCESS: Your Handout has been Created!"));
?>
Solution
You have a major security vulnerability.
As in, serious enough that if I knew your web address, I could delete your entire database.
Near the top of your code, you set
As of rit now, if I send a POST request to this page with the
the following query will be executed on your database:
The first query will throw a syntax error, the second will delete your database, and any subsequent queries are commented out.
You need to fix this before you do anything else.
As in, serious enough that if I knew your web address, I could delete your entire database.
Near the top of your code, you set
$cid to the raw value of $_POST["cid"]. That's bad. You need to escape this data before you put it anywhere near a database.As of rit now, if I send a POST request to this page with the
cid parameter set to ; DROP DATABASE Inventory; --, your entire database is gone. That string will be put into the $cid variable, and then when you execute these lines:$query = "SELECT id FROM Handouts WHERE name='$name' AND cid=$cid";
$result = mysqli_query($sql, $query);the following query will be executed on your database:
SELECT id FROM Handouts WHERE name='' AND cid=; DROP DATABASE Inventory; --The first query will throw a syntax error, the second will delete your database, and any subsequent queries are commented out.
You need to fix this before you do anything else.
Code Snippets
$query = "SELECT id FROM Handouts WHERE name='$name' AND cid=$cid";
$result = mysqli_query($sql, $query);SELECT id FROM Handouts WHERE name='' AND cid=; DROP DATABASE Inventory; --Context
StackExchange Code Review Q#126287, answer score: 6
Revisions (0)
No revisions yet.