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

Secure INSERTs with MySQLi

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