Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > comp.lang.php > #3804 > unrolled thread

i getting this warning

Back to article view | Back to comp.lang.php

Contents

  i getting this warning sri kanth <sritullimilli@gmail.com> - 2011-11-16 03:17 -0800
    Re: i getting this warning The Natural Philosopher <tnp@invalid.invalid> - 2011-11-16 11:26 +0000
      Re: i getting this warning tony@mountifield.org (Tony Mountifield) - 2011-11-16 11:37 +0000
    Re: i getting this warning Jerry Stuckle <jstucklex@attglobal.net> - 2011-11-16 06:56 -0500
      Re: i getting this warning Denis McMahon <denismfmcmahon@gmail.com> - 2011-11-16 15:11 +0000
        Re: i getting this warning Jerry Stuckle <jstucklex@attglobal.net> - 2011-11-16 10:41 -0500
        Re: i getting this warning Arno Welzel <usenet@arnowelzel.de> - 2011-11-17 14:31 +0100
          Re: i getting this warning Arno Welzel <usenet@arnowelzel.de> - 2011-11-17 14:34 +0100
            Re: i getting this warning Denis McMahon <denismfmcmahon@gmail.com> - 2011-11-18 08:02 +0000
          Re: i getting this warning The Natural Philosopher <tnp@invalid.invalid> - 2011-11-17 13:39 +0000
          Re: i getting this warning Jerry Stuckle <jstucklex@attglobal.net> - 2011-11-17 09:01 -0500
            Re: i getting this warning Arno Welzel <usenet@arnowelzel.de> - 2011-11-17 15:15 +0100
              Re: i getting this warning Jerry Stuckle <jstucklex@attglobal.net> - 2011-11-17 09:44 -0500
                Re: i getting this warning Arno Welzel <usenet@arnowelzel.de> - 2011-11-21 14:59 +0100
                  Re: i getting this warning Jerry Stuckle <jstucklex@attglobal.net> - 2011-11-21 09:12 -0500

#3804 — i getting this warning

mysql_fetch_row(): supplied argument is not a valid MySQL result
below my code:
<?php
$qs=$_REQUEST['id'];
mysql_connect("localhost","root","");
mysql_select_db("test");
$data=mysql_query("select * from tbl_porduct where pid=$qs");
$rec=mysql_fetch_row($data);
$pname=$rec[1];
$price=$rec[3];
$sid=session_id();
mysql_query("insert into tbl_spro values('$sid','$pname','$price')");
?>

[toc] | [next] | [standalone]

#3805

sri kanth wrote:
> mysql_fetch_row(): supplied argument is not a valid MySQL result
> below my code:
> <?php
> $qs=$_REQUEST['id'];
> mysql_connect("localhost","root","");
> mysql_select_db("test");
> $data=mysql_query("select * from tbl_porduct where pid=$qs");
> $rec=mysql_fetch_row($data);
> $pname=$rec[1];
> $price=$rec[3];
> $sid=session_id();
> mysql_query("insert into tbl_spro values('$sid','$pname','$price')");
> ?>
> 
I am not surprised

$data=mysql_query("select * from tbl_porduct where pid=$qs");

is not going to win friends and influence people

Try
$data=mysql_query(sprintf("select * from tbl_porduct where pid=%d",$qs));

and check whether 'tbl_porduct' should in fact be 'tbl_product'.

[toc] | [prev] | [next] | [standalone]

#3806

In article <ja06lf$3b4$2@news.albasani.net>,
The Natural Philosopher  <tnp@invalid.invalid> wrote:
> sri kanth wrote:
> > mysql_fetch_row(): supplied argument is not a valid MySQL result
> > below my code:
> > <?php
> > $qs=$_REQUEST['id'];
> > mysql_connect("localhost","root","");
> > mysql_select_db("test");
> > $data=mysql_query("select * from tbl_porduct where pid=$qs");
> > $rec=mysql_fetch_row($data);
> > $pname=$rec[1];
> > $price=$rec[3];
> > $sid=session_id();
> > mysql_query("insert into tbl_spro values('$sid','$pname','$price')");
> > ?>
> > 
> I am not surprised
> 
> $data=mysql_query("select * from tbl_porduct where pid=$qs");
> 
> is not going to win friends and influence people
> 
> Try
> $data=mysql_query(sprintf("select * from tbl_porduct where pid=%d",$qs));
> 
> and check whether 'tbl_porduct' should in fact be 'tbl_product'.

And also test the return value in $data before using it to try and fetch
a row.

Then test $rec to see if it actually returned you a matching row at all.

You should actually test the return status of ALL your mysql_ calls, and
not just blindly assume they will work.

Cheers
Tony
-- 
Tony Mountifield
Work: tony@softins.co.uk - http://www.softins.co.uk
Play: tony@mountifield.org - http://tony.mountifield.org

[toc] | [prev] | [next] | [standalone]

#3807

On 11/16/2011 6:17 AM, sri kanth wrote:
> mysql_fetch_row(): supplied argument is not a valid MySQL result
> below my code:
> <?php
> $qs=$_REQUEST['id'];
> mysql_connect("localhost","root","");
> mysql_select_db("test");
> $data=mysql_query("select * from tbl_porduct where pid=$qs");
> $rec=mysql_fetch_row($data);
> $pname=$rec[1];
> $price=$rec[3];
> $sid=session_id();
> mysql_query("insert into tbl_spro values('$sid','$pname','$price')");
> ?>
>

Three things.

First of all, you probably have an error in your SQL some place - it 
could be in the connect, selecting the database or the query itself. 
But you aren't checking the results of any of your MySQL calls, so you 
don't know which might be failing.

Never assume a call works; always check the return values - see the PHP 
manual for return values.  Then if the call fails, print out a message 
and log the error (see mysql_error()).  On a development system you can 
display the error, but you do not want to do so on a production system.

Second, are you really using "root" with no password?  If you are, 
change it immediately.  This is a huge security whole and can leave your 
database open to all kinds of bad stuff.

Third, never do "SELECT *" from a database.  Always specify the columns 
you wish to fetch.  There are two reasons for this.  First of all, what 
if you (or someone else) adds a 5 MB BLOB column to the table?  You'll 
be returning a lot of information that you're not interested in for this 
request.  Specifying the columns means you won't return that BLOB unless 
you specifically ask for it.

More importantly, a change in the database can cause very difficult to 
locate problems.  For instance, lets say you have a column "name" which 
contains both first and last names.  Later you find you need to change 
this to "first_name, last_name".  Your SELECT * will still work, but 
later in your code (maybe much later) you won't have the information you 
expect in the places you expect it - this can be a very hard problem to 
troubleshoot.  Specifying the column names in your query will cause the 
query to fail in the above case, making the problem obvious.

And as a side note, if you use mysql_fetch_assoc() instead of 
mysql_fetch_row, you can use the column names as the index, i.e. 
$row['name'] instead of $row[1].  It will make your code easier to read 
and understand (plus the returned columns are not position dependent).

-- 
==================
Remove the "x" from my email address
Jerry Stuckle
JDS Computer Training Corp.
jstucklex@attglobal.net
==================

[toc] | [prev] | [next] | [standalone]

#3809

On Wed, 16 Nov 2011 06:56:21 -0500, Jerry Stuckle wrote:

> On 11/16/2011 6:17 AM, sri kanth wrote:

>> $qs=$_REQUEST['id'];
>> $data=mysql_query("select * from tbl_porduct where pid=$qs");

> Three things.

You missed "using unescaped user input in a query with no validation or 
verification". I know it's only a select, but would you bet that he's 
that sloppy with selects and yet rigorous with data changing statements?

I suspect his code would do something unexpected (by him anyway) if I 
sent a get for http://host/page?id=*, as I suspect he only expects the 
query to return a single row. ;)

Perhaps a check that the number of rows returned by the query was however 
many he expected after checking that the query didn't fail would be a 
good thing too?

Rgds

Denis McMahon

[toc] | [prev] | [next] | [standalone]

#3810

On 11/16/2011 10:11 AM, Denis McMahon wrote:
> On Wed, 16 Nov 2011 06:56:21 -0500, Jerry Stuckle wrote:
>
>> On 11/16/2011 6:17 AM, sri kanth wrote:
>
>>> $qs=$_REQUEST['id'];
>>> $data=mysql_query("select * from tbl_porduct where pid=$qs");
>
>> Three things.
>
> You missed "using unescaped user input in a query with no validation or
> verification". I know it's only a select, but would you bet that he's
> that sloppy with selects and yet rigorous with data changing statements?
>
> I suspect his code would do something unexpected (by him anyway) if I
> sent a get for http://host/page?id=*, as I suspect he only expects the
> query to return a single row. ;)
>
> Perhaps a check that the number of rows returned by the query was however
> many he expected after checking that the query didn't fail would be a
> good thing too?
>
> Rgds
>
> Denis McMahon

Good point, Denis!

-- 
==================
Remove the "x" from my email address
Jerry Stuckle
JDS Computer Training Corp.
jstucklex@attglobal.net
==================

[toc] | [prev] | [next] | [standalone]

#3811

Denis McMahon, 2011-11-16 16:11:

> On Wed, 16 Nov 2011 06:56:21 -0500, Jerry Stuckle wrote:
> 
>> On 11/16/2011 6:17 AM, sri kanth wrote:
> 
>>> $qs=$_REQUEST['id'];
>>> $data=mysql_query("select * from tbl_porduct where pid=$qs");
> 
>> Three things.
> 
> You missed "using unescaped user input in a query with no validation or 
> verification". I know it's only a select, but would you bet that he's 
> that sloppy with selects and yet rigorous with data changing statements?

It does not matter what statement there *is*. Using data from outside in
this way makes *everything* possible - this is the typical mistake which
makes SQL injection possible!


Example:

Lets assume $qs is "1;drop tlb_product".

$data = mysql_query("select * from tbl_product where pid=$qs");

The statement will be expanded to:

"select * from tbl_product where pid=1;drop tbl_product"

The result will be, that the table tbl_product will be dropped, if the
MySQL user has the right to drop tables.


So:

1) Do never trust data from outside

2) Always check results and do never assume successful execution


A better solution may be (requires at least PHP 5) - not tested (may
contain typos):

try
{
  if(!isset($_REQUEST['id']))
    throw new Exception('parameter id missing');

  $qs = $_REQUEST['id'];

  if(!is_numeric($qs))
    throw new Exception('parameter id not numeric');

  if(!mysql_connect('localhost', 'root', ''))
    throw new Exception('can not connect to database');

  if(!mysql_select_db('test'))
    throw new Exception('can not connect to database');

  $statement = sprintf("select * from tbl_product where pid=%d", $qs);

  $data = mysql_query($statement);
  if(!$data)
    throw new Exception('no product found');

  $rec = mysql_fetch_row($data);
  if(!$rec)
    throw new Exception('error fetching product record');

  $pname=$rec[1];
  $price=$rec[3];
  $sid=session_id();

  if(!mysql_query(
    "insert into tbl_spro values('$sid','$pname','$price')")
    throw new Exception('can not store values');
} catch (Exception $e) {
  // Handle exception here, $e->getMessage() will
  // return the messages provided above
}


-- 
Arno Welzel
http://arnowelzel.de
http://de-rec-fahrrad.de

[toc] | [prev] | [next] | [standalone]

#3812

Arno Welzel, 2011-11-17 14:31:

[...]
> 1) Do never trust data from outside
> 
> 2) Always check results and do never assume successful execution


Sorry - forgot this:

3) Do NOT use "root" to connect to the MySQL database and limit the
access rights for the MySQL user as far as possible - if you never need
to create or drop tables within your own scripts, then the user should
not have the right for it. The same applies to any other operation
within the database.


-- 
Arno Welzel
http://arnowelzel.de
http://de-rec-fahrrad.de

[toc] | [prev] | [next] | [standalone]

#3823

On Thu, 17 Nov 2011 14:34:54 +0100, Arno Welzel wrote:

> Sorry - forgot this:
> 
> 3) Do NOT use "root" ...

Yes, Jerry already covered that point.

You seem to have joined in half way through the discussion, I suggest you 
go back to the start and see who said what.

Rgds

Denis McMahon

[toc] | [prev] | [next] | [standalone]

#3813

Arno Welzel wrote:
> Denis McMahon, 2011-11-16 16:11:
> 
>> On Wed, 16 Nov 2011 06:56:21 -0500, Jerry Stuckle wrote:
>>
>>> On 11/16/2011 6:17 AM, sri kanth wrote:
>>>> $qs=$_REQUEST['id'];
>>>> $data=mysql_query("select * from tbl_porduct where pid=$qs");
>>> Three things.
>> You missed "using unescaped user input in a query with no validation or 
>> verification". I know it's only a select, but would you bet that he's 
>> that sloppy with selects and yet rigorous with data changing statements?
> 
> It does not matter what statement there *is*. Using data from outside in
> this way makes *everything* possible - this is the typical mistake which
> makes SQL injection possible!
> 
> 
> Example:
> 
> Lets assume $qs is "1;drop tlb_product".
> 
> $data = mysql_query("select * from tbl_product where pid=$qs");
> 
> The statement will be expanded to:
> 
> "select * from tbl_product where pid=1;drop tbl_product"
> 
> The result will be, that the table tbl_product will be dropped, if the
> MySQL user has the right to drop tables.
> 

which is why I always use sprintf("...id='%d'", $id)....

whatever ends up in 'id=' is always numeric and that's all it is.

and even if its a string. if you encapsulate it in quotes, then it cant 
go outside the bounds of the query.




> 
> So:
> 
> 1) Do never trust data from outside
> 
+1

> 2) Always check results and do never assume successful execution
> 
>

Unless you don't mind it failing.

[toc] | [prev] | [next] | [standalone]

#3814

On 11/17/2011 8:31 AM, Arno Welzel wrote:
> Denis McMahon, 2011-11-16 16:11:
>
>> On Wed, 16 Nov 2011 06:56:21 -0500, Jerry Stuckle wrote:
>>
>>> On 11/16/2011 6:17 AM, sri kanth wrote:
>>
>>>> $qs=$_REQUEST['id'];
>>>> $data=mysql_query("select * from tbl_porduct where pid=$qs");
>>
>>> Three things.
>>
>> You missed "using unescaped user input in a query with no validation or
>> verification". I know it's only a select, but would you bet that he's
>> that sloppy with selects and yet rigorous with data changing statements?
>
> It does not matter what statement there *is*. Using data from outside in
> this way makes *everything* possible - this is the typical mistake which
> makes SQL injection possible!
>
>
> Example:
>
> Lets assume $qs is "1;drop tlb_product".
>
> $data = mysql_query("select * from tbl_product where pid=$qs");
>
> The statement will be expanded to:
>
> "select * from tbl_product where pid=1;drop tbl_product"
>
> The result will be, that the table tbl_product will be dropped, if the
> MySQL user has the right to drop tables.
>
>
<snip>

The statement will fail because mysql_query() will not execute multiple 
statements in a single query.

-- 
==================
Remove the "x" from my email address
Jerry Stuckle
JDS Computer Training Corp.
jstucklex@attglobal.net
==================

[toc] | [prev] | [next] | [standalone]

#3815

Jerry Stuckle, 2011-11-17 15:01:

> On 11/17/2011 8:31 AM, Arno Welzel wrote:
>> Denis McMahon, 2011-11-16 16:11:
>>
>>> On Wed, 16 Nov 2011 06:56:21 -0500, Jerry Stuckle wrote:
>>>
>>>> On 11/16/2011 6:17 AM, sri kanth wrote:
>>>
>>>>> $qs=$_REQUEST['id'];
>>>>> $data=mysql_query("select * from tbl_porduct where pid=$qs");
>>>
>>>> Three things.
>>>
>>> You missed "using unescaped user input in a query with no validation or
>>> verification". I know it's only a select, but would you bet that he's
>>> that sloppy with selects and yet rigorous with data changing statements?
>>
>> It does not matter what statement there *is*. Using data from outside in
>> this way makes *everything* possible - this is the typical mistake which
>> makes SQL injection possible!
>>
>>
>> Example:
>>
>> Lets assume $qs is "1;drop tlb_product".
>>
>> $data = mysql_query("select * from tbl_product where pid=$qs");
>>
>> The statement will be expanded to:
>>
>> "select * from tbl_product where pid=1;drop tbl_product"
>>
>> The result will be, that the table tbl_product will be dropped, if the
>> MySQL user has the right to drop tables.
>>
>>
> <snip>
> 
> The statement will fail because mysql_query() will not execute multiple 
> statements in a single query.

Generally and in this specific case you are right - but it is possible
and you should never rely on this behaviour.

See also: <http://php.net/manual/de/function.mysql-query.php>


-- 
Arno Welzel
http://arnowelzel.de
http://de-rec-fahrrad.de

[toc] | [prev] | [next] | [standalone]

#3816

On 11/17/2011 9:15 AM, Arno Welzel wrote:
> Jerry Stuckle, 2011-11-17 15:01:
>
>> On 11/17/2011 8:31 AM, Arno Welzel wrote:
>>> Denis McMahon, 2011-11-16 16:11:
>>>
>>>> On Wed, 16 Nov 2011 06:56:21 -0500, Jerry Stuckle wrote:
>>>>
>>>>> On 11/16/2011 6:17 AM, sri kanth wrote:
>>>>
>>>>>> $qs=$_REQUEST['id'];
>>>>>> $data=mysql_query("select * from tbl_porduct where pid=$qs");
>>>>
>>>>> Three things.
>>>>
>>>> You missed "using unescaped user input in a query with no validation or
>>>> verification". I know it's only a select, but would you bet that he's
>>>> that sloppy with selects and yet rigorous with data changing statements?
>>>
>>> It does not matter what statement there *is*. Using data from outside in
>>> this way makes *everything* possible - this is the typical mistake which
>>> makes SQL injection possible!
>>>
>>>
>>> Example:
>>>
>>> Lets assume $qs is "1;drop tlb_product".
>>>
>>> $data = mysql_query("select * from tbl_product where pid=$qs");
>>>
>>> The statement will be expanded to:
>>>
>>> "select * from tbl_product where pid=1;drop tbl_product"
>>>
>>> The result will be, that the table tbl_product will be dropped, if the
>>> MySQL user has the right to drop tables.
>>>
>>>
>> <snip>
>>
>> The statement will fail because mysql_query() will not execute multiple
>> statements in a single query.
>
> Generally and in this specific case you are right - but it is possible
> and you should never rely on this behaviour.
>
> See also:<http://php.net/manual/de/function.mysql-query.php>
>
>

You are preaching to the choir here.  I'm just pointing out the error in 
your comments.

If you had been reading this newsgroup for the past 8 years or so, you 
will find many of us (including Denis and myself) have long been 
proponents of this.

But you obviously failed to understand the discussion.

-- 
==================
Remove the "x" from my email address
Jerry Stuckle
JDS Computer Training Corp.
jstucklex@attglobal.net
==================

[toc] | [prev] | [next] | [standalone]

#3837

Jerry Stuckle, 2011-11-17 15:44:

> On 11/17/2011 9:15 AM, Arno Welzel wrote:
>> Jerry Stuckle, 2011-11-17 15:01:
>>
>>> On 11/17/2011 8:31 AM, Arno Welzel wrote:
>>>> Denis McMahon, 2011-11-16 16:11:
>>>>
>>>>> On Wed, 16 Nov 2011 06:56:21 -0500, Jerry Stuckle wrote:
>>>>>
>>>>>> On 11/16/2011 6:17 AM, sri kanth wrote:
>>>>>
>>>>>>> $qs=$_REQUEST['id'];
>>>>>>> $data=mysql_query("select * from tbl_porduct where pid=$qs");
>>>>>
>>>>>> Three things.
>>>>>
>>>>> You missed "using unescaped user input in a query with no validation or
>>>>> verification". I know it's only a select, but would you bet that he's
>>>>> that sloppy with selects and yet rigorous with data changing statements?
>>>>
>>>> It does not matter what statement there *is*. Using data from outside in
>>>> this way makes *everything* possible - this is the typical mistake which
>>>> makes SQL injection possible!
>>>>
>>>>
>>>> Example:
>>>>
>>>> Lets assume $qs is "1;drop tlb_product".
>>>>
>>>> $data = mysql_query("select * from tbl_product where pid=$qs");
>>>>
>>>> The statement will be expanded to:
>>>>
>>>> "select * from tbl_product where pid=1;drop tbl_product"
>>>>
>>>> The result will be, that the table tbl_product will be dropped, if the
>>>> MySQL user has the right to drop tables.
>>>>
>>>>
>>> <snip>
>>>
>>> The statement will fail because mysql_query() will not execute multiple
>>> statements in a single query.
>>
>> Generally and in this specific case you are right - but it is possible
>> and you should never rely on this behaviour.
>>
>> See also:<http://php.net/manual/de/function.mysql-query.php>
>>
>>
> 
> You are preaching to the choir here.  I'm just pointing out the error in 
> your comments.

And i already agreed with you. So what's your point?

And just tried to explain why the assumption that using multiple queries
is not a problem, since mysql_query() would fail anyway, may be wrong.

> If you had been reading this newsgroup for the past 8 years or so, you 
> will find many of us (including Denis and myself) have long been 
> proponents of this.
> 
> But you obviously failed to understand the discussion.

Then ignore my statements.


-- 
Arno Welzel
http://arnowelzel.de
http://de-rec-fahrrad.de

[toc] | [prev] | [next] | [standalone]

#3839

On 11/21/2011 8:59 AM, Arno Welzel wrote:
> Jerry Stuckle, 2011-11-17 15:44:
>
>> On 11/17/2011 9:15 AM, Arno Welzel wrote:
>>> Jerry Stuckle, 2011-11-17 15:01:
>>>
>>>> On 11/17/2011 8:31 AM, Arno Welzel wrote:
>>>>> Denis McMahon, 2011-11-16 16:11:
>>>>>
>>>>>> On Wed, 16 Nov 2011 06:56:21 -0500, Jerry Stuckle wrote:
>>>>>>
>>>>>>> On 11/16/2011 6:17 AM, sri kanth wrote:
>>>>>>
>>>>>>>> $qs=$_REQUEST['id'];
>>>>>>>> $data=mysql_query("select * from tbl_porduct where pid=$qs");
>>>>>>
>>>>>>> Three things.
>>>>>>
>>>>>> You missed "using unescaped user input in a query with no validation or
>>>>>> verification". I know it's only a select, but would you bet that he's
>>>>>> that sloppy with selects and yet rigorous with data changing statements?
>>>>>
>>>>> It does not matter what statement there *is*. Using data from outside in
>>>>> this way makes *everything* possible - this is the typical mistake which
>>>>> makes SQL injection possible!
>>>>>
>>>>>
>>>>> Example:
>>>>>
>>>>> Lets assume $qs is "1;drop tlb_product".
>>>>>
>>>>> $data = mysql_query("select * from tbl_product where pid=$qs");
>>>>>
>>>>> The statement will be expanded to:
>>>>>
>>>>> "select * from tbl_product where pid=1;drop tbl_product"
>>>>>
>>>>> The result will be, that the table tbl_product will be dropped, if the
>>>>> MySQL user has the right to drop tables.
>>>>>
>>>>>
>>>> <snip>
>>>>
>>>> The statement will fail because mysql_query() will not execute multiple
>>>> statements in a single query.
>>>
>>> Generally and in this specific case you are right - but it is possible
>>> and you should never rely on this behaviour.
>>>
>>> See also:<http://php.net/manual/de/function.mysql-query.php>
>>>
>>>
>>
>> You are preaching to the choir here.  I'm just pointing out the error in
>> your comments.
>
> And i already agreed with you. So what's your point?
>
> And just tried to explain why the assumption that using multiple queries
> is not a problem, since mysql_query() would fail anyway, may be wrong.
>
>> If you had been reading this newsgroup for the past 8 years or so, you
>> will find many of us (including Denis and myself) have long been
>> proponents of this.
>>
>> But you obviously failed to understand the discussion.
>
> Then ignore my statements.
>
>

No problem.  Consider yourself ignorant.

-- 
==================
Remove the "x" from my email address
Jerry Stuckle
JDS Computer Training Corp.
jstucklex@attglobal.net
==================

[toc] | [prev] | [standalone]

Back to top | Article view | comp.lang.php

csiph-web