0

I am trying to execute an INSERT INTO query but for some reason an error is thrown.

$result1 = mysqli_query($connection, 
             "INSERT INTO `results` 
                     (`result_quiz_name`, `result_marks`, `result_grade`, 
                      `student_id`, `result_max_marks`) 
              VALUES ('Ionic Bonding Introduction', $marks, $grade, 
                $student, 5)");

The variable $marks is an integer, the $grade variable is a string and the $student variable is just the integer value of a session variable. The error that is displayed is this:

Error: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near ' 1, 5)' at line 1

I just can't seem to work out where the syntax error is. I have run this query in phpMyAdmin and it works fine (obviously substituting the variables for values).

Thanks,

RiggsFolly
  • 93,638
  • 21
  • 103
  • 149
Th3Gam3rz
  • 171
  • 1
  • 3
  • 11
  • 1
    Strings need to be quoted. You are probably open to SQL injections with this code. You should use parameterized queries. – chris85 Nov 07 '15 at 15:05
  • 1
    am I open to SQL injections even if the variables that I am using are not inputted by the user but are only calculated by what multi choice option they click? – Th3Gam3rz Nov 07 '15 at 15:11
  • A user can make their own request to your script with the same variable name and whatever content they want to send. Once your script gets that request it would process it and you are injected. Additionally a user could manipulate the DOM and send malicious content that way. – chris85 Nov 07 '15 at 15:15
  • Because I am using POST variables could someone make their own request? Also, if someone was to say manipulate one of the hidden input tags that I use for the answers then it would not matter because all I do in my code is check if the inputted answer is the correct answer, and if it is then the $marks variable is increased. I might be wrong though. I did think about using prepared statements and pdo, but I was not sure if it would be necessary. – Th3Gam3rz Nov 07 '15 at 15:19
  • 1
    That answer being correct feature just makes the injection harder to occur; still open to it. `GET` or `POST` doesn't matter you should never pass user input direct to your SQL. Take a look at 1. http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php?rq=1, 2. http://php.net/manual/en/security.database.sql-injection.php 3. https://www.owasp.org/index.php/SQL_Injection_Prevention_Cheat_Sheet#Defense_Option_1:_Prepared_Statements_.28Parameterized_Queries.29 – chris85 Nov 07 '15 at 15:24

5 Answers5

3

Just like in PHP — in SQL a string must be quoted, and quotes inside of string syntax must be properly escaped if they are a part of the value.

Use var_dump($marks, $grade, $student) to inspect your variables. If $grade is a string like you say then you really should be running it through the proper escape functions (i.e. mysqli_real_escape_string in your case) for your database.

Even better is to just use parameterized queries, which both PDO and MySQLi will support in PHP.

So for example you can change your sql code to the following...

$stmt = mysqli_prepare($connection, 
             "INSERT INTO `results` 
                     (`result_quiz_name`, `result_marks`, `result_grade`, 
                      `student_id`, `result_max_marks`) 
              VALUES ('Ionic Bonding Introduction', ?, ?, ?, 5)");

Which gives you a prepared statement that's safe for inserting any value without breaking your SQL syntax...

if ($stmt) {
    $stmt->bind_param("i", $marks);
    $stmt->bind_param("s", $grade);
    $stmt->bind_param("i", $student);
    $stmt->execute(); // this executes your statement
    $result = $stmt->get_result(); // this gives you the result set
    $row = $result->fetch_array(); // get rows from there
}
Sherif
  • 11,786
  • 3
  • 32
  • 57
  • You are totally right in your distain of my answer. However if my 23K has taught me anything it is that 99.9% of the questioners, especially the weekend coders, will _ignore all the preaching about the right way to do things, especially if it involves them in major changes or leaning something new_ and take the path of least resistance regardless. I have attempted to deleted my answer in favour of yours, but cannot as it was accepted, so lets hope the questioner accepts your suggestions and your answer. – RiggsFolly Nov 07 '15 at 17:21
  • @RiggsFolly while I generally agree that you can lead a horse to water, but can't make it drink - I still believe there is value in raising the bar on SO. Don't get me wrong, I totally admire your candor here and thank you for demonstrating equal smarts and humility. – Sherif Nov 07 '15 at 17:32
-1

Try using following code

  $result1 = mysqli_query($connection, "INSERT INTO `results` (`result_quiz_name`, `result_marks`, `result_grade`, `student_id`, `result_max_marks`) VALUES ('Ionic Bonding Introduction', '$marks', '$grade', '$student', 5)");
  • Alvin, welcome to the stack. Try to add a little Alvin verbiage to Answers to suggest the problem encountered. A little eye candy here and there goes a long way toward votes. I know it sounds ridiculous but trust me on that one – Drew Nov 07 '15 at 15:35
-1

Change $grade to '$grade'

$result1 = mysqli_query($connection, 
             "INSERT INTO `results` 
                     (`result_quiz_name`, `result_marks`, `result_grade`, 
                      `student_id`, `result_max_marks`) 
              VALUES ('Ionic Bonding Introduction', $marks, '$grade', 
                $student, 5)");
William
  • 2,695
  • 1
  • 21
  • 33
-1

When a column type is text/string/char/varchar etc you have to wrap the variable in quotes in your parameter list, just like you did for 'Ionic Bonding Introduction'

$result1 = mysqli_query($connection, 
         "INSERT INTO `results` 
                 (`result_quiz_name`, `result_marks`, `result_grade`, 
                  `student_id`, `result_max_marks`) 
          VALUES ('Ionic Bonding Introduction', $marks, '$grade', 
            $student, 5)");
RiggsFolly
  • 93,638
  • 21
  • 103
  • 149
  • I expect these rep-seeking flat answers from people who are new on SO, but for someone with 23K rep? It's a sad day when this becomes the accepted answer with not even an etiolated attempt to address the most common faux pas the op will most likely run into immediately after using such a solution. – Sherif Nov 07 '15 at 15:23
  • **FAIL**. [https://www.owasp.org/index.php/SQL_Injection](https://www.owasp.org/index.php/SQL_Injection) – spencer7593 Nov 07 '15 at 16:13
-1

Since $grade is a string, it should be quoted in the query:

$result1 = mysqli_query($connection, "INSERT INTO `results` (`result_quiz_name`, `result_marks`, `result_grade`, `student_id`, `result_max_marks`) VALUES ('Ionic Bonding Introduction', $marks, '$grade', $student, 5)");
coder.in.me
  • 1,048
  • 9
  • 19