Possible SQL injection vulnerability searching for a product by ID?

I want to be sure that this isn't vulnerable to SQL injection. If yes, then how can it be improved?

$pid = $_REQUEST['product_id'];
$query1 = "SELECT * FROM products WHERE id = '" 
  . $pid . "'";

$result = mysql_db_query($dbname, $query1) or die("Failed Query of " . $query1);
$thisrow=mysql_fetch_row($result);
if ($thisrow) {
  echo "The Product was found.";
}
else
{
  echo "The Product was not found.";
}

On demand -> issues mentioned in comments, reviewed & posted as an answer:

The answer to your question has already been given: Yes, you have a serious vulnerability, and the first step to solving this is using the extensions that aren't deprecated, like PDO and mysqli.

Both of these extensions support the easiest, and pretty solid (not perfect, but solid) prodection from injection attacks: prepared statements. Learn to use them, learn to love them.

The rest of your code is, I'm afraid to say pretty bad. A lot of the issues are the result of your using the mysql_* extension, but there are other issues, too.

A line-by-line review pointing out issues as we go:

$pid = $_REQUEST['product_id'];

This statement can, and probably will at some point, produce notices. If the product_id parameter isn't set, you'll get an "undefined index" notice. When writing, and testing, new code, always set your error_reporting to E_STRICT | E_ALL (which, if you're running php 5.5+ will show E_DEPRECATED notices for your using the mysql_* extension). And set display_errors to true, so you can See the problems. Anyway, always check if an index exists, before using it:

$pid = isset($_REQUEST['product_id']) ? $_REQUEST['product_id'] : null;

Next - Concatenating raw values into a query is never a good idea. Your query appears to have id = '' as a WHERE clause, which suggests that id is a string, but that wouldn't make sense. You're also assuming that the client will behave nicely, and not request a product_id value that contains single quotes (which would mess up your query!). Nor do you seem to mind negative values, or any other non-valid input for that matter. Worst of all, its laughably easy for me to send a request that'll result in a valid query that performs a full table scan (a products table can be quite large, so this would be something that can be exploited in a DOS attack):

//the product_id value:
$pid = "1' OR '1";
//the resulting query would be
$query1 = "SELECT * FROM products WHERE id = '1' OR '1'";
//which, basically amounts to:
$query1 = 'SELECT * FROM products';

It's quite obvious that this is not what you want, you don't want your code to echo The product was found when you're actually querying to see if any product exists. Especially if that query basically selects all records in your table.

So this statement needs to be reworked:

$query1 = "SELECT * FROM products WHERE id = '" 
. $pid . "'";

Given that you're using an extension that doesn't support prepared statements, the best thing you can do is actually casting the values, and using printf to insert the value of $pid in your query string.

Also: avoid SELECT * whenever possible. Select the fields you need, and name them. Be explicit.

I'd also add some basic validation, though like is_numeric, to make sure the request parameter is a valid id value. If not, the client might attempting an injection attack. Anyway, the simple fix here would be:

$queryString = sprintf(
    'SELECT * FROM products WHERE id = %d',
    (int) $pid
);

I left out the validation, that's code you'll have to write yourself first...

Onwards - Making the resource parameter optional in mysql was, IMHO, a sure fire way to breed bad practices, and error-prone code. You've fallen into this trap... don't worry, we all have at some point, we've all written the infamous or die, and used mysql_query assuming that the last active connection would be the connection we needed. But most of us have found out the hard way that this isn't always the case... So please, fix this bit, too:

$result = mysql_db_query($dbname, $query1)
    or die("Failed Query of " . $query1);

The or die is just smelly, IMHO. I don't like code that just gives up like that, wrap this snippet in a function, and replace the or die with throwing an exception, so that you can handle the error a bit cleaner than just dumping a query onto the screen (which is showing that the product_id parameter is used in this query, so it's basically sending an injection-attack-invitation).

You are also not passing a connection resource to mysql_db_query. I know it's an optional parameter, but you'd think it'd be nice to know what connection is actually being used to query this product stuff, if only to show that you actually know what DB this table is on, and to prevent bugs popping up, when you start adding multiple connections to the project.

$thisrow=mysql_fetch_row($result);

Again, in light of my aversion for or die constructs, I'd much rather see code actually bothering to check the return value of functions, like here: checking if $result is a resource, or if it's false or even true. If $result is true, your or die statement won't stop the code from executing, BTW. I know it's not very likely to happen in this case, but you never know, if, down the road, the SELECT gets replaced with an INSERT or

StackExchange Code Review Q#70238, answer score: 17