i getting this warning [message #176014] |
Wed, 16 November 2011 11:17 |
sritullimilli
Messages: 13 Registered: November 2011
Karma: 0
|
Junior Member |
|
|
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')");
?>
|
|
|
Re: i getting this warning [message #176015 is a reply to message #176014] |
Wed, 16 November 2011 11:26 |
The Natural Philosoph
Messages: 993 Registered: September 2010
Karma: 0
|
Senior Member |
|
|
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'.
|
|
|
Re: i getting this warning [message #176016 is a reply to message #176015] |
Wed, 16 November 2011 11:37 |
tony
Messages: 19 Registered: December 2010
Karma: 0
|
Junior Member |
|
|
In article <ja06lf$3b4$2(at)news(dot)albasani(dot)net>,
The Natural Philosopher <tnp(at)invalid(dot)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(at)softins(dot)co(dot)uk - http://www.softins.co.uk
Play: tony(at)mountifield(dot)org - http://tony.mountifield.org
|
|
|
Re: i getting this warning [message #176017 is a reply to message #176014] |
Wed, 16 November 2011 11:56 |
Jerry Stuckle
Messages: 2598 Registered: September 2010
Karma: 0
|
Senior Member |
|
|
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(at)attglobal(dot)net
==================
|
|
|
Re: i getting this warning [message #176019 is a reply to message #176017] |
Wed, 16 November 2011 15:11 |
Denis McMahon
Messages: 634 Registered: September 2010
Karma: 0
|
Senior Member |
|
|
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
|
|
|
Re: i getting this warning [message #176020 is a reply to message #176019] |
Wed, 16 November 2011 15:41 |
Jerry Stuckle
Messages: 2598 Registered: September 2010
Karma: 0
|
Senior Member |
|
|
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(at)attglobal(dot)net
==================
|
|
|
Re: i getting this warning [message #176021 is a reply to message #176019] |
Thu, 17 November 2011 13:31 |
Arno Welzel
Messages: 317 Registered: October 2011
Karma: 0
|
Senior Member |
|
|
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
|
|
|
Re: i getting this warning [message #176022 is a reply to message #176021] |
Thu, 17 November 2011 13:34 |
Arno Welzel
Messages: 317 Registered: October 2011
Karma: 0
|
Senior Member |
|
|
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
|
|
|
Re: i getting this warning [message #176023 is a reply to message #176021] |
Thu, 17 November 2011 13:39 |
The Natural Philosoph
Messages: 993 Registered: September 2010
Karma: 0
|
Senior Member |
|
|
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.
|
|
|
Re: i getting this warning [message #176024 is a reply to message #176021] |
Thu, 17 November 2011 14:01 |
Jerry Stuckle
Messages: 2598 Registered: September 2010
Karma: 0
|
Senior Member |
|
|
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(at)attglobal(dot)net
==================
|
|
|
Re: i getting this warning [message #176025 is a reply to message #176024] |
Thu, 17 November 2011 14:15 |
Arno Welzel
Messages: 317 Registered: October 2011
Karma: 0
|
Senior Member |
|
|
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
|
|
|
Re: i getting this warning [message #176026 is a reply to message #176025] |
Thu, 17 November 2011 14:44 |
Jerry Stuckle
Messages: 2598 Registered: September 2010
Karma: 0
|
Senior Member |
|
|
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(at)attglobal(dot)net
==================
|
|
|
Re: i getting this warning [message #176033 is a reply to message #176022] |
Fri, 18 November 2011 08:02 |
Denis McMahon
Messages: 634 Registered: September 2010
Karma: 0
|
Senior Member |
|
|
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
|
|
|
Re: i getting this warning [message #176043 is a reply to message #176026] |
Mon, 21 November 2011 13:59 |
Arno Welzel
Messages: 317 Registered: October 2011
Karma: 0
|
Senior Member |
|
|
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
|
|
|
Re: i getting this warning [message #176045 is a reply to message #176043] |
Mon, 21 November 2011 14:12 |
Jerry Stuckle
Messages: 2598 Registered: September 2010
Karma: 0
|
Senior Member |
|
|
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(at)attglobal(dot)net
==================
|
|
|