HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Adding an item to database + JSON

Submitted by: @import:stackexchange-codereview··
0
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 $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.