-1
$ID                 = $db->real_escape_string(strip_tags(stripslashes($_GET['ID'])));
            $GetThreadFromID    = mysqli_fetch_object(mysqli_query($db, "SELECT * FROM ForumThreads WHERE ID=$ID"));
            $GetThreadStarter   = mysqli_fetch_object(mysqli_query($db, "SELECT * FROM Users WHERE ID='$GetThreadFromID->PosterID'"));
            $GetTopicFromThread = mysqli_fetch_object(mysqli_query($db, "SELECT * FROM ForumTopics WHERE ID='$GetThreadFromID->ForumID'"));
            $ThreadExist        = mysqli_num_rows(mysqli_query($db, "SELECT * FROM ForumThreads WHERE ID='$ID'"));
            $GetAllWatching     = mysqli_query($db, "SELECT * FROM ForumWatchedThreads WHERE ThreadID='$ID' AND UserID='$client->ID'");

            if ($ThreadExist == "0") {
                                echo "
                                <div class='container'>
                                    <div class='panel panel-danger'>
                                        <div class='panel-heading'>Error</div>
                                        <div class='panel-body'>
                                            The thread you requested does not exist.
                                        </div>
                                    </div>
                                </div>
                                ";
            include $_SERVER["DOCUMENT_ROOT"]."/_INCLUDES/Footer.php";
            exit();
            }

I have a website with a hand-coded forum. You can access forum threads fine. Say the thread ID is 12, but I put an apostrophe at the end of it in ?ID= then the forum title and forum body will be blank along with other information. How can I make it so it will display the thread with the ID, so if I put this in the URL bar: ?ID=13// or ?ID=13'', it will still display ?ID=13 without any interference, or even display an error saying that the forum thread does not exist?

Joe F.
  • 9
  • 1
  • Read this http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php?rq=1 – rjdown Aug 19 '15 at 19:25

1 Answers1

1

You're simply assuming the queries are succeeding. Since you're NOT checking for failure, your == "0" will SUCCEED if the query FAILS, as the boolean FALSE returned by query() and fetch() will test as equal to "0".

NEVER assume success with a db operation. Even if your SQL syntax is perfect, there's a near infinite number of reasons for the query to fail anyways.

$result = mysqli_query(...) or die(mysqli_error($db));
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^

if (mysqli_num_rows($result) === 0) {
  die("No results");
}

The or die() will handle the query failing outright, and the === strict equality test will ensure that you're testing for an actual integer 0, and not any OTHER value that also tests equal to 0, e.g. '' == 0 is TRUE in php.

And note that your first query is STILL vulnerable to some forms of sql injection attacks, in spite of all the stripping and quoting you're doing. e.g. 1 or 1=1 will slip right through, because there's no tags, no slashes, and no ' characters to escape.

Marc B
  • 356,200
  • 43
  • 426
  • 500
  • Thank you so much for the explanation. How can I fix my queries that may be vulnerable to SQL injection, and are they actually vulnerable for someone to get into my database right now if they wanted to? I just did '$ID' instead of plain ID in the first query, which fixed the blank displaying. Did this also fix the SQL injection? – Joe F. Aug 19 '15 at 19:35
  • you're using mysqli. use prepared statements and placeholders: http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php – Marc B Aug 19 '15 at 19:35
  • That means I would have to rewrite every single query on my site, which will be very annoying. What do I do? – Joe F. Aug 19 '15 at 19:36
  • better get busy, or you will suffer for it later. – Marc B Aug 19 '15 at 19:38
  • I have used a program to try to sql inject my pages but I haven't gotten into the database yet. Why is that? – Joe F. Aug 19 '15 at 19:41
  • because you're not vulnerable to insert/update injection attacks, because mysqli uses a driver which doesn't allow multiple queries in a single query() call. but you ARE vulnerable to stuff like authentication bypasses. – Marc B Aug 19 '15 at 20:04