2

Before anybody says, I will protect myself against SQL injections, right after I fix this error. I am making an app where news reports are submitted to the database. This page is what removes a report from the database.

What I have tried: Every possible way of adding brackets to the SQL and speech marks. My ICT teacher and I have looked at this for nearly 2 hours and cannot find a fix. I have also searched Google and Stack Overflow but I cannot find an answer.

Ok, so the correct report_id displays when I echo it. When I put the actual id, eg 5, the report is deleted. But when I put $report_id, nothing is deleted.

Please could somebody tell me what correction I have to make to get this to work ?

Here is the code (EDIT: This is the fixed code. I added the hidden field in the form at the bottom, among a few other small changes (like taking out the extra form tag)):

<?php
  require_once('authorize.php');
?>

<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN"
  "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en">
<head>
  <meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
  <title>Football Central - Remove a Report</title>
</head>
<body>
  <h2>Football Central - Remove a News Report</h2>

<?php
  require_once('img_details_reports.php');
  require_once('connect_db_reports.php');

   //Assign variables from admin_reports.php using $_GET
   $report_id = $_GET['id'];

    if (isset($_POST['submit'])) {
     if ($_POST['confirm'] == 'Yes') {

      $report_id = $_POST['id'];
      // Delete the image file from the server
      @unlink(IMAGE_UPLOADPATH . $image);

      // Connect to the database
      $dbc = mysqli_connect(DB_HOST, DB_USER, DB_PASSWORD, DB_NAME)
      or die("Unable to connect to the database."); 

      // Delete the score data from the database
      $query = "DELETE FROM news_reports WHERE report_id = '".$report_id."' LIMIT 1" 
      or die("mysql_query failed - Error: " . mysqli_error());

      mysqli_query($dbc, $query) or die("mysql_query failed - Error: " . mysqli_error());
      mysqli_close($dbc);
     }
    }         

    //Display form to confirm delete
    echo '<p>Are you sure you want to delete the news report?</p>';
    echo '<form method="post" action="removereport.php">';
    echo '<input type="radio" name="confirm" value="Yes" /> Yes ';
    echo '<input type="radio" name="confirm" value="No" checked="checked" /> No <br />';
    echo '<input type="hidden" name="id" value="' . $report_id . '" />';
    echo '<input type="submit" value="Submit" name="submit" />';
    echo '</form>';

    echo '<p><a href="admin_reports.php">&lt;&lt; Back to admin reports page</a></p>';
?>

</body>
</html>
lcolli98
  • 169
  • 1
  • 3
  • 18
  • 7
    +1 for pre-empting boring SQL injection sermon – imulsion May 17 '13 at 16:42
  • 1
    How do you send the `id` variable? – SAVAFA May 17 '13 at 16:44
  • I presume your gets var, submit, confirm and id exists, right....try making an echo of every one of them at the begin of your php script – Hackerman May 17 '13 at 16:48
  • Whenever I send queries to MYSQL I end them with a semi-colon. Not sure if that's necessary. Give it a go? Also, what is the error code? What do you see if you print `$query`? – Jimbo May 17 '13 at 16:49
  • 1
    wait... what are you doing here? A form in a form?! – bwoebi May 17 '13 at 16:55
  • Thank you SAVAFA you fixed it :D I needed to add it hidden in the form. Sorry I don't know why I have a nested form, stupid mistake :) – lcolli98 May 20 '13 at 17:57
  • @lukecolli98, did I solve it or someone else (based on your selected answer, which previously was mine)? ;) – SAVAFA May 20 '13 at 21:15
  • Oh sorry, I hadn't seen my stack for a few days and there were so many answers :D I changed your answer to the solve thing. I have just updated the code as well. – lcolli98 May 21 '13 at 19:28

5 Answers5

3

Your are mixing two statements. Just try below.

  // Delete the score data from the database
  $query = "DELETE FROM news_reports WHERE report_id = ".$report_id;  
  mysqli_query($dbc, $query) or die("mysql_query failed - Error: " . mysqli_error($dbc));
Rikesh
  • 26,156
  • 14
  • 79
  • 87
  • 1
    +1. This is the correct answer. the `die` statement is misplaced – Matanya May 17 '13 at 16:56
  • @Matanya That's right, but a string should evaluate to boolean true, so this won't cause the problem... – bwoebi May 17 '13 at 16:58
  • Is it correct for sending the database link, see it in here: http://php.net/manual/en/mysqli.query.php – SAVAFA May 17 '13 at 17:00
  • @bwoebi, You are right. lcolli98, can you `var_dump($query)` and show us the output? – Matanya May 17 '13 at 17:03
  • Hi, everything is fixed after I added the ID into the form ! When I use POST, the ID is not retrieved, only with GET. Do you know why this is ? – lcolli98 May 20 '13 at 17:56
0

You are sending the form with post method and retrieving it with get. That can be the source of the problem.

Also you are not sending the id parameter so, there won't be any value for $_get[id] nor $_post[id]

SAVAFA
  • 818
  • 8
  • 23
  • but I don't see any input name="id" there? – bwoebi May 17 '13 at 16:49
  • The default form method is GET ([see this answer](http://stackoverflow.com/questions/2314401/what-is-the-default-form-posting-method)), but your point about the lack of an `id` id valid. – George Cummins May 17 '13 at 16:52
  • you are right about the default, but he is setting the method as `post` already – SAVAFA May 17 '13 at 16:54
  • Ohhh. He has a nested form. There is a form header two lines before without a `method=`. – George Cummins May 17 '13 at 17:04
  • Hi, thanks for the help. I added the input id into the form and it works great ! – lcolli98 May 20 '13 at 17:53
  • @lukecolli98, update your codes in the initial so that I can take a look at it. – SAVAFA May 20 '13 at 21:16
  • @lukecolli98, why do you have both of these lines: `$report_id = $_GET['id'];` and `$report_id = $_POST['id'];`, and what is the problem after all the modifications? – SAVAFA May 21 '13 at 21:36
  • @SAVAFA, That's just a mistake. The only problem now is that everyone here said I should change everything to POST instead of GET, but when I do this the ID is not retrieved ? – lcolli98 May 23 '13 at 05:53
  • How do you know that the `id` is not retrieved by `$_POST['id']`? Your online example just works fine and the row is removed. – SAVAFA May 23 '13 at 11:11
0

You shouldn't have to wrap the ID in single-quotes, if the ID is a number.

$query = "DELETE FROM news_reports WHERE report_id = '".$report_id."' LIMIT 1"

But that's not the problem. You did not include the ID in your confirmation request, or allow for retrieving the value from a session variable. Add a hidden input box with the id, in your "Display form to confirm delete" section.

(And have a different code branch for confirmation! And a test for an invalid ID! And move this to POST, at the very least!)

DougM
  • 2,808
  • 17
  • 14
  • Hi thanks for that ! When I use POST, the ID is not retrieved, only with GET. Do you know why this is ? – lcolli98 May 20 '13 at 19:28
0

You've got

$query = "..." or die(...);

Why?

Also, you've got two form opening tags -- it's not valid to nest forms.

I'm going to assume that the id variable comes in from some source other than the form because there's no form element that submits it.

Finally, make sure to specify get or post in your form. I'd recommend using post, and then change $_GET["submit"] and $_GET["confirm"] to $_POST["submit"] and $_POST["confirm"].

Carson Myers
  • 37,678
  • 39
  • 126
  • 176
  • Hi, the or die was to see if I could get the exact problem :) When I use POST, the ID is not retrieved, only with GET. Do you know why this is ? – lcolli98 May 20 '13 at 17:52
0

You need to check following things in your code.

  • Where is your ID element in the form
  • You have put POST method in form but retrieving data from $_GET, you should change it to $_POST.
  • Put mysqli_error after mysqli_query statement.

    $query = "DELETE FROM news_reports WHERE report_id = ".$report_id;
    
    mysqli_query($dbc, $query);  or die("mysql_query failed - Error: " . mysqli_error());
    

Then check the error from mysql if it does not work.

Hope, it will help you in solving your issue.

Nishu Tayal
  • 20,106
  • 8
  • 49
  • 101
  • Hi, I added the ID into the form and it works great ! When I use POST, the ID is not retrieved, only with GET. Do you know why this is ? – lcolli98 May 20 '13 at 17:54