[phpesp-dev] [ phpesp-Bugs-2128464 ] Unquoted variable ($_name) allows SQL injection attack

[SourceForge.net <no...@so...>]

Bugs item #2128464, was opened at 2008-09-25 09:54
Message generated for change (Comment added) made by bishopb
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=108956&aid=2128464&group_id=8956

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: SQL
Group: None
>Status: Closed
>Resolution: Invalid
Priority: 1
Private: No
Submitted By: bishop (bishopb)
>Assigned to: bishop (bishopb)
Summary: Unquoted variable ($_name) allows SQL injection attack

Initial Comment:
Reported via email to Matthew Gregg:

File: phpESP/public/survey.php
Lines:

15  $_name = _addslashes($_GET['name']);
25  $_sql = "SELECT id,title,theme FROM " $GLOBALS['ESPCONFIG']['survey_table']." WHERE name = $_name";

Since the variable $_name is not embedded in quotes, the function addslashes will not prevent SQL injection attacks since the attacker does not need to use quotes.

PoC:
survey.php?name=1 and 1=0 union select null,username, password from designer

Fix:
25  $_sql = "SELECT id,title,theme FROM ".$GLOBALS['ESPCONFIG']['survey_table']." WHERE name = '$_name'";

----------------------------------------------------------------------

>Comment By: bishop (bishopb)
Date: 2008-09-25 09:58

Message:
$_name is quoted by _addslashes().  Here is the call order:
 _addslashes() calls db_qstr()
 db_qstr() calls ADODB::qstr()
 ADODB::qstr() does the proper quoting to prevent SQL injection attacks.
 
So, as long as the variables are going through _addslashes(), then there
is no bug.

Had _addslashes() not been present, the proposed fix (just enclosing in
single quotes) is itself insufficient, as single quotes can be fooled by
prematurely closing the quote, inserting a statement, then restarting, as
in:

'; DELETE FROM respondent; '1=1

Requirement: all parameters to all SQL statements should go through
_addslashes()

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=108956&aid=2128464&group_id=8956