Difficulty passing GET variable?

Question

Hi I am trying to display specific entries in a database by appending the variable name to a URL like:

echo '<td><a class="index_table" href="includes/view.php?id=$row[id]>$row[Orderno]">

and then in my view.php I have:

<?php
include 'connect.php';
//Display the Data//
$id=$_GET['id']; 
$result=mysql_query("select * from Products where ID=$id");
$row=mysql_fetch_object($result);
echo "<table>";
echo "
<tr bgcolor='#f1f1f1'><td><b>ID</b></td><td>$row->ID</td></tr>

However the specific ID is not being passed to the script, and the table in view.php is blank. When changing the where clause to 'where id = '1' the correct product displays. So I know that this is working.

Many Thanks

but I suppose you acces it by : `http://site.com/view.php?id=23` right? — Alex, Dec 17 '12 at 16:08
What if I pass this parameter: includes/view.php?id=1%20or%20true — Diodeus - James MacFarlane, Dec 17 '12 at 16:09
Also it's vulnerable to sql injection Most answers contain SQL injection! Take care with that! You need first to cast $id as and integer! — Jimmy Kane, Dec 17 '12 at 16:10
What happens when i decide to send this to your server `includes/view.php?id=1%3BDROP%20TABLE%20PRODUCTS%3B` ? — gbtimmon, Dec 17 '12 at 16:44
I think the problem is that **you arent encoding the $_GET parameter**. View this topic: **http://stackoverflow.com/a/15642716/2131877** because you need to encode the parameters to pass it as one variable. — T.Todua, Mar 26 '13 at 16:55

score 5 · Accepted Answer · answered Dec 17 '12 at 16:09

Basic PHP syntax: Strings quoted with ' do not interpolate variable values:

echo '<td><a class="index_table" href="includes/view.php?id=' . $row['id'] . '>' . $row['Orderno'] . '">';
                                                            ^^^^^^^^^^^^^^^^^^

note that you're wide open to SQL injection attacks and are just begging to get your server pwn3d.

shadyyx · Answer 2 · 2012-12-17T16:17:05.587

First problem:

You have to put the array string indexes into a paranthesis:

echo '<td><a class="index_table" href="includes/view.php?id='.$row['id'].'">'.$row['Orderno'].'</a></td>';
                                                            ^^^^^^^^^^^^^^  ^^^^^^^^^^^^^^^^^^^

Second problem:

Your ID in the URL could easily be replaced with '; DELETE FROM table # thus allowing an attacker to perform a SQL injection! Always sanitize any user input (POST) or GET parameters that takes a part in SQL queries:

$id = mysql_real_escape_string($_GET['id']);

or for that case (when an integer is expected)

$id = (int) $_GET['id'];

Suggestion: do not use mysql_* functions but use PDO with (real!) prepared statements or at least mysqli_* functions with proper input sanitization.

score 1 · Answer 3 · answered Dec 17 '12 at 16:14

Two big issues here. First, your link is not working correctly because you are using single-quotes in your echo, meaning the variables are not interpolated, so you must change to something like either of the following:

echo "<td><a class=\"index_table\" href=\"includes/view.php?id={$row['id']}>{$row['Orderno']}\">";

or

echo '<td><a class="index_table" href="includes/view.php?id=' . $row['id'] . '>' . $row['Orderno'] . '">';

WARNING - SECURITY BREACH

In your later code you are leaving yourself open to SQL Injection attack; some references to what this is can be found at OWASP and Wikipedia, and are very important to learn about. To protect yourself, you must escape data before sending it to a query. Here are some ways to do that:

$id = mysql_real_escape_string($_GET['id']); 
$result=mysql_query("select * from Products where ID = '$id'");

or

$id = $_GET['id']; 
if (!ctype_digit((string)$id)) {
    die('Invalid ID: ' . htmlentities($id));
}
$result=mysql_query("select * from Products where ID = '$id'");

In the first example, I use mysql_real_escape_string to make the data safe for embedding in a query (note that I also added quotes around the variable); in the second, I did a data check to make sure it contained only digits (note that the length should also be checked, but this is a quick example), and if it contained something other than digits, we spit out an error message and don't run the query.

Why would you escape by hand ??? PHP supports prepared statements, there is no reason I can think of to escape by hand over prepared statements... — gbtimmon, Dec 17 '12 at 16:50
Submitter is already using the old mysql driver, we don't know what his/her environment looks like, whether PDO is installed, or what capabilities s/he has to do upgrades, if any. It is far and away simpler to answer in the style that solves the problem(s) at hand without completely shifting paradigms -- if I was chatter with the submitter face-to-face, I'd recommend PDO in a second, but this format does not lend itself too well to such things. — Dereleased, Dec 17 '12 at 19:05

score 0 · Answer 4 · answered Dec 17 '12 at 16:09

0

Change your query like, I added two ' between $id

$result=mysql_query("select * from Products where ID='$id'");

And see.

answered Dec 17 '12 at 16:09

someone

6,577
7
37
60

score 0 · Answer 5 · edited May 23 '17 at 12:13

You're not actually including the value of the $id variable in the query. Take a look at this answer for options on how to do this:

How can I prevent SQL injection in PHP?

PDO

$stmt = $pdo->prepare('SELECT * FROM employees WHERE name = :name');

$stmt->execute(array(':name' => $name));

foreach ($stmt as $row) {
    // do something with $row
}

mysqli

$stmt = $dbConnection->prepare('SELECT * FROM employees WHERE name = ?');
$stmt->bind_param('s', $name);

$stmt->execute();

$result = $stmt->get_result();
while ($row = $result->fetch_assoc()) {
    // do something with $row
}

score 0 · Answer 6 · answered Dec 17 '12 at 16:10

You shouldn't put the GET variable directly into the query like that, you should do some sanity checks like checking it's numeric etc. to avoid sql injection.

No doubt you will have answers saying the mysql_ functions are deprecated aswell but I don't think that's relevant to the question.

In your link you have

<td><a class="index_table" href="includes/view.php?id=$row[id]>$row[Orderno]">

you don't have the right syntax for the array elements, try

<td><a class="index_table" href="includes/view.php?id=' . $row['id'] . '>' . $row['Orderno'] . '">

score 0 · Answer 7 · answered Dec 17 '12 at 16:11

It looks like a malformed URL in the tag, plus PHP doesn't parse variables in single quoted strings. I think you just need this:

echo "<td><a class='index_table' href='includes/view.php?id=$row[id]'>$row[Orderno]</a></td>";

You don't need to change the code in view.php but I would recommend filtering the _GET variable this way:

$id = (int)$_GET['id'];

SnareChops · Answer 8 · 2012-12-17T16:41:04.533

Try

echo "<td><a class='index_table' href='includes/view.php?id=".$row['id'].">".$row['Orderno']."'>";

and

<?php
include 'connect.php';
//Display the Data//
$id=$_GET['id']; 
if(is_int($id))
{
    $result=mysql_query("select * from Products where ID=$id");
    $row=mysql_fetch_object($result);
    echo "<table>";
    echo "<tr bgcolor='#f1f1f1'><td><b>ID</b></td><td>$row->ID</td></tr>";
}
else
{
    echo "<h1>Nice try silly... You aint hackin me!</h1>";
}

I also noticed in your original code you were missing some ending quotes and semi-colons. That may have been all that was wrong. But this should clear up your security issue and should work for your application

Good luck.

Difficulty passing GET variable?

8 Answers8

WARNING - SECURITY BREACH