patternphpMinor
Secure INSERTs with MySQLi
Viewed 0 times
insertssecurewithmysqli
Problem
Is this code well protected, and if not, could you tell me how it might be exploited and how to secure it?
if(isset($_POST['vrsta_predmeta']) AND !empty($_POST['vrsta_predmeta']) AND
isset($_POST['res_text']) AND isset($_POST['glavni_dug']) AND isset($_POST['res']) AND isset($_POST['zaklj']) AND isset($_POST['povjerilac']) AND isset($_POST['duznik']) AND isset($_POST['predmet_zaveden'])){
$racunob = trim($_POST['rac']);
$obrazlozenje = trim($_POST['obr']);
$ob_text = trim($_POST['res_ob']);
$res_text = trim($_POST['res_text']);
$vrsta_pre = trim($_POST['vrsta_predmeta']);
$izvrsenje = trim(strtolower($_POST['res']));
$obrazac = trim($_POST['zaklj']);
$povjerilac = $_POST['povjerilac'];
$duznik = $_POST['duznik'];
$datum= trim($_POST['predmet_zaveden']);
foreach($povjerilac as $key){
$lica = $db -> prepare("INSERT INTO p_lica(povjerilac, doc_br, dokument_vlasnik) VALUES('$key', '$dok_broj', '$ses_val')");
}
foreach($duznik as $key1){
$lica1 = $db -> prepare("INSERT INTO d_lica(duznik,doc_br, dokument_vlasnik) VALUES('$key1', '$dok_broj', '$ses_val')");
}
$insert_dok = $db -> prepare("INSERT INTO document_tbl(dokument_vlasnik,dokument_broj,vrsta_dokumenta,zakljucak, resenje_izvrsenja,datum,resenje_text,obrazlozenje,obtext,racunob) VALUES('$ses_val','$dok_broj', '$vrsta_pre','$obrazac','$izvrsenje','$datum','$res_text','$obrazlozenje','$ob_text','$racunob')");
if($lica -> execute() AND $insert_dok -> execute() AND $lica1 -> execute()){
$lica -> close();
$lica1 -> close();
$insert_dok -> close();
echo 'new Messi(\'Dokument uspjesno dodat.\', {title: \'Obavjestenje\', titleClass: \'success\', buttons: [{id: 0, label: \'Close\', val: \'X\'}]});';
header('location:login.php');
}else{
echo 'new Messi(\'Dokument uspjesno dodat.\', {title: \'Obavjestenje\', titleClass: \'anim warning\', buttons: [{id: 0, label: \'Close\', val: \'X\'}]});';
}
}Solution
No, this code is not secure, as you are using prepared statements wrong. See here for correct usage of mysqli prepared statements (you should not ever have user supplied input in the
For example, your code might look like this:
(see also here or here)
Misc
prepare statement, it is only for SQL syntax; add the user input via bind later on).For example, your code might look like this:
$sql = $db->prepare('INSERT INTO p_lica (povjerilac, doc_br, dokument_vlasnik) VALUES(?, ?, ?)'); // note that there is no user input in prepare
$sql->bind_param('iss', $key, $dok_broj, $ses_val); // s for string, i for integer. use d for double, b for blob
foreach($povjerilac as $key) { // set values for $key, which is bound to the first ? parameter
$sql->execute(); // insert new row with current key
}
$sql->close();(see also here or here)
Misc
- formatting: your code is very badly formatted. If this is just a copy-paste error it's ok, but otherwise, you should definitely fix it.
- variable names: the standard is to use english names, if you can.
- your header call seems wrong. From the documentation:
Remember that header() must be called before any actual output is sent. You should also use a complete URL.
Code Snippets
$sql = $db->prepare('INSERT INTO p_lica (povjerilac, doc_br, dokument_vlasnik) VALUES(?, ?, ?)'); // note that there is no user input in prepare
$sql->bind_param('iss', $key, $dok_broj, $ses_val); // s for string, i for integer. use d for double, b for blob
foreach($povjerilac as $key) { // set values for $key, which is bound to the first ? parameter
$sql->execute(); // insert new row with current key
}
$sql->close();Context
StackExchange Code Review Q#68078, answer score: 3
Revisions (0)
No revisions yet.