From: Matthew G. <mat...@gm...> - 2008-09-25 13:05:38
|
I received the message below, but don't have time to do a thorough investigation at the moment. A quick look, seem like this is not a problem. Anyone with more time please take a look. > 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'"; |
From: Franky V. L. <lie...@te...> - 2008-09-25 13:22:26
|
IIRC the function _addslashes adds quotes itself, therefore no quotes are needed in the sql statement. I'll check this evening, but for now I would advise no action. Franky On Thu, Sep 25, 2008 at 3:05 PM, Matthew Gregg <mat...@gm...>wrote: > I received the message below, but don't have time to do a thorough > investigation at the moment. A quick look, seem like this is not a > problem. Anyone with more time please take a look. > > > 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'"; > > > ------------------------------------------------------------------------- > This SF.Net email is sponsored by the Moblin Your Move Developer's > challenge > Build the coolest Linux based applications with Moblin SDK & win great > prizes > Grand prize is a trip for two to an Open Source event anywhere in the world > http://moblin-contest.org/redirect.php?banner_id=100&url=/ > _______________________________________________ > phpESP-devel mailing list > php...@li... > https://lists.sourceforge.net/lists/listinfo/phpesp-devel > > |
From: Matthew G. <mat...@gm...> - 2008-09-25 13:24:20
|
That is what I thought as well, just wanted more eyes on this. On Thu, 2008-09-25 at 15:22 +0200, Franky Van Liedekerke wrote: > IIRC the function _addslashes adds quotes itself, therefore no quotes > are needed in the sql statement. I'll check this evening, but for now > I would advise no action. > > Franky > > On Thu, Sep 25, 2008 at 3:05 PM, Matthew Gregg > <mat...@gm...> wrote: > I received the message below, but don't have time to do a > thorough > investigation at the moment. A quick look, seem like this is > not a > problem. Anyone with more time please take a look. > > > 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'"; > > > ------------------------------------------------------------------------- > This SF.Net email is sponsored by the Moblin Your Move > Developer's challenge > Build the coolest Linux based applications with Moblin SDK & > win great prizes > Grand prize is a trip for two to an Open Source event anywhere > in the world > http://moblin-contest.org/redirect.php?banner_id=100&url=/ > _______________________________________________ > phpESP-devel mailing list > php...@li... > https://lists.sourceforge.net/lists/listinfo/phpesp-devel > > > > ------------------------------------------------------------------------- > This SF.Net email is sponsored by the Moblin Your Move Developer's challenge > Build the coolest Linux based applications with Moblin SDK & win great prizes > Grand prize is a trip for two to an Open Source event anywhere in the world > http://moblin-contest.org/redirect.php?banner_id=100&url=/ > _______________________________________________ phpESP-devel mailing list php...@li... https://lists.sourceforge.net/lists/listinfo/phpesp-devel |
From: Bishop B. <ph...@id...> - 2008-09-25 13:51:17
|
Regarding my last email, I skipped over the fact that $_name is going through _addslashes(), so my point about the bug being valid is false. 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. bishop Quoting Franky Van Liedekerke <lie...@te...>: > IIRC the function _addslashes adds quotes itself, therefore no quotes are > needed in the sql statement. I'll check this evening, but for now I would > advise no action. > > Franky > > On Thu, Sep 25, 2008 at 3:05 PM, Matthew Gregg > <mat...@gm...>wrote: > >> I received the message below, but don't have time to do a thorough >> investigation at the moment. A quick look, seem like this is not a >> problem. Anyone with more time please take a look. >> >> > 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'"; >> >> >> ------------------------------------------------------------------------- >> This SF.Net email is sponsored by the Moblin Your Move Developer's >> challenge >> Build the coolest Linux based applications with Moblin SDK & win great >> prizes >> Grand prize is a trip for two to an Open Source event anywhere in the world >> http://moblin-contest.org/redirect.php?banner_id=100&url=/ >> _______________________________________________ >> phpESP-devel mailing list >> php...@li... >> https://lists.sourceforge.net/lists/listinfo/phpesp-devel >> >> > -- Bishop Bettini ideacode, Inc. (main) +1 919 341 5170 / (fax) +1 919 521 4100 Visit us on the web at: ideacode.com Professional software research and development reviewmysoftware.com Improve sales! Review your software before you release bytejar.com Solutions to those annoying development problems |
From: Matthew G. <mat...@gm...> - 2008-09-25 13:28:40
|
Isn't $_name quoted? _addslashes does the quoting. if(get_magic_quotes_gpc()) { function _addslashes($a) { return(db_qstr(stripslashes($a))); } function _stripslashes($a) { return(stripslashes($a)); } } else { function _addslashes($a) { return(db_qstr($a)); } function _stripslashes($a) { return($a); } } On Thu, 2008-09-25 at 09:24 -0400, Bishop Bettini wrote: > Any parameters to an SQL query not going through the adodb quoting > mechanism is vulnerable to SQL injection attacks. 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 > > So, the problem is legitimate, the fix is not. A bug (or task) should > be added to tracker to go through all SQL commands and ensure all > parameters are quoted, including this instance. Thoughts? > > bishop > > > Quoting Matthew Gregg <mat...@gm...>: > > > I received the message below, but don't have time to do a thorough > > investigation at the moment. A quick look, seem like this is not a > > problem. Anyone with more time please take a look. > > > >> 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'"; > > > > > > ------------------------------------------------------------------------- > > This SF.Net email is sponsored by the Moblin Your Move Developer's challenge > > Build the coolest Linux based applications with Moblin SDK & win great prizes > > Grand prize is a trip for two to an Open Source event anywhere in the world > > http://moblin-contest.org/redirect.php?banner_id=100&url=/ > > _______________________________________________ > > phpESP-devel mailing list > > php...@li... > > https://lists.sourceforge.net/lists/listinfo/phpesp-devel > > > > > |
From: Bishop B. <ph...@id...> - 2008-09-25 13:51:18
|
Yeah, I missed the fact that $_name went through _addslashes() in the original email. I just now followed up with a trace verifying _addslashes() quotes properly. Nonetheless, I'd advise adding the incoming message as a bug, then marking it as not a bug, so we know this issue's been handled. bishop Quoting Matthew Gregg <mat...@gm...>: > Isn't $_name quoted? _addslashes does the quoting. > > if(get_magic_quotes_gpc()) { > function _addslashes($a) > { return(db_qstr(stripslashes($a))); } > function _stripslashes($a) { return(stripslashes($a)); } > } else { > function _addslashes($a) { return(db_qstr($a)); } > function _stripslashes($a) { return($a); } > } > > On Thu, 2008-09-25 at 09:24 -0400, Bishop Bettini wrote: >> Any parameters to an SQL query not going through the adodb quoting >> mechanism is vulnerable to SQL injection attacks. 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 >> >> So, the problem is legitimate, the fix is not. A bug (or task) should >> be added to tracker to go through all SQL commands and ensure all >> parameters are quoted, including this instance. Thoughts? >> >> bishop >> >> >> Quoting Matthew Gregg <mat...@gm...>: >> >> > I received the message below, but don't have time to do a thorough >> > investigation at the moment. A quick look, seem like this is not a >> > problem. Anyone with more time please take a look. >> > >> >> 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'"; >> > >> > >> > ------------------------------------------------------------------------- >> > This SF.Net email is sponsored by the Moblin Your Move >> Developer's challenge >> > Build the coolest Linux based applications with Moblin SDK & win >> great prizes >> > Grand prize is a trip for two to an Open Source event anywhere in >> the world >> > http://moblin-contest.org/redirect.php?banner_id=100&url=/ >> > _______________________________________________ >> > phpESP-devel mailing list >> > php...@li... >> > https://lists.sourceforge.net/lists/listinfo/phpesp-devel >> > >> >> >> > > -- Bishop Bettini ideacode, Inc. (main) +1 919 341 5170 / (fax) +1 919 521 4100 Visit us on the web at: ideacode.com Professional software research and development reviewmysoftware.com Improve sales! Review your software before you release bytejar.com Solutions to those annoying development problems |
From: Matthew G. <mat...@gm...> - 2008-09-25 13:36:40
|
I think a bug for this is a good idea. I know many moons ago I did this, but lots of code has changed. On Thu, 2008-09-25 at 09:32 -0400, Bishop Bettini wrote: > Yeah, I missed the fact that $_name went through _addslashes() in the > original email. I just now followed up with a trace verifying > _addslashes() quotes properly. > > Nonetheless, I'd advise adding the incoming message as a bug, then > marking it as not a bug, so we know this issue's been handled. > > bishop > > Quoting Matthew Gregg <mat...@gm...>: > > > Isn't $_name quoted? _addslashes does the quoting. > > > > if(get_magic_quotes_gpc()) { > > function _addslashes($a) > > { return(db_qstr(stripslashes($a))); } > > function _stripslashes($a) { return(stripslashes($a)); } > > } else { > > function _addslashes($a) { return(db_qstr($a)); } > > function _stripslashes($a) { return($a); } > > } > > > > On Thu, 2008-09-25 at 09:24 -0400, Bishop Bettini wrote: > >> Any parameters to an SQL query not going through the adodb quoting > >> mechanism is vulnerable to SQL injection attacks. 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 > >> > >> So, the problem is legitimate, the fix is not. A bug (or task) should > >> be added to tracker to go through all SQL commands and ensure all > >> parameters are quoted, including this instance. Thoughts? > >> > >> bishop > >> > >> > >> Quoting Matthew Gregg <mat...@gm...>: > >> > >> > I received the message below, but don't have time to do a thorough > >> > investigation at the moment. A quick look, seem like this is not a > >> > problem. Anyone with more time please take a look. > >> > > >> >> 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'"; > >> > > >> > > >> > ------------------------------------------------------------------------- > >> > This SF.Net email is sponsored by the Moblin Your Move > >> Developer's challenge > >> > Build the coolest Linux based applications with Moblin SDK & win > >> great prizes > >> > Grand prize is a trip for two to an Open Source event anywhere in > >> the world > >> > http://moblin-contest.org/redirect.php?banner_id=100&url=/ > >> > _______________________________________________ > >> > phpESP-devel mailing list > >> > php...@li... > >> > https://lists.sourceforge.net/lists/listinfo/phpesp-devel > >> > > >> > >> > >> > > > > > > > |
From: Bishop B. <ph...@id...> - 2008-09-25 14:01:02
|
I added and resolved the bug. May want to forward to the original reporter. bishop Quoting Matthew Gregg <mat...@gm...>: > I think a bug for this is a good idea. I know many moons ago I did > this, but lots of code has changed. > > On Thu, 2008-09-25 at 09:32 -0400, Bishop Bettini wrote: >> Yeah, I missed the fact that $_name went through _addslashes() in the >> original email. I just now followed up with a trace verifying >> _addslashes() quotes properly. >> >> Nonetheless, I'd advise adding the incoming message as a bug, then >> marking it as not a bug, so we know this issue's been handled. >> >> bishop >> >> Quoting Matthew Gregg <mat...@gm...>: >> >> > Isn't $_name quoted? _addslashes does the quoting. >> > >> > if(get_magic_quotes_gpc()) { >> > function _addslashes($a) >> > { return(db_qstr(stripslashes($a))); } >> > function _stripslashes($a) { return(stripslashes($a)); } >> > } else { >> > function _addslashes($a) { return(db_qstr($a)); } >> > function _stripslashes($a) { return($a); } >> > } >> > >> > On Thu, 2008-09-25 at 09:24 -0400, Bishop Bettini wrote: >> >> Any parameters to an SQL query not going through the adodb quoting >> >> mechanism is vulnerable to SQL injection attacks. 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 >> >> >> >> So, the problem is legitimate, the fix is not. A bug (or task) should >> >> be added to tracker to go through all SQL commands and ensure all >> >> parameters are quoted, including this instance. Thoughts? >> >> >> >> bishop >> >> >> >> >> >> Quoting Matthew Gregg <mat...@gm...>: >> >> >> >> > I received the message below, but don't have time to do a thorough >> >> > investigation at the moment. A quick look, seem like this is not a >> >> > problem. Anyone with more time please take a look. >> >> > >> >> >> 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'"; >> >> > >> >> > >> >> > >> ------------------------------------------------------------------------- >> >> > This SF.Net email is sponsored by the Moblin Your Move >> >> Developer's challenge >> >> > Build the coolest Linux based applications with Moblin SDK & win >> >> great prizes >> >> > Grand prize is a trip for two to an Open Source event anywhere in >> >> the world >> >> > http://moblin-contest.org/redirect.php?banner_id=100&url=/ >> >> > _______________________________________________ >> >> > phpESP-devel mailing list >> >> > php...@li... >> >> > https://lists.sourceforge.net/lists/listinfo/phpesp-devel >> >> > >> >> >> >> >> >> >> > >> > >> >> >> > > > ------------------------------------------------------------------------- > This SF.Net email is sponsored by the Moblin Your Move Developer's challenge > Build the coolest Linux based applications with Moblin SDK & win great prizes > Grand prize is a trip for two to an Open Source event anywhere in the world > http://moblin-contest.org/redirect.php?banner_id=100&url=/ > _______________________________________________ > phpESP-devel mailing list > php...@li... > https://lists.sourceforge.net/lists/listinfo/phpesp-devel > -- Bishop Bettini ideacode, Inc. (main) +1 919 341 5170 / (fax) +1 919 521 4100 Visit us on the web at: ideacode.com Professional software research and development reviewmysoftware.com Improve sales! Review your software before you release bytejar.com Solutions to those annoying development problems |
From: Bishop B. <ph...@id...> - 2008-09-25 13:51:17
|
Any parameters to an SQL query not going through the adodb quoting mechanism is vulnerable to SQL injection attacks. 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 So, the problem is legitimate, the fix is not. A bug (or task) should be added to tracker to go through all SQL commands and ensure all parameters are quoted, including this instance. Thoughts? bishop Quoting Matthew Gregg <mat...@gm...>: > I received the message below, but don't have time to do a thorough > investigation at the moment. A quick look, seem like this is not a > problem. Anyone with more time please take a look. > >> 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'"; > > > ------------------------------------------------------------------------- > This SF.Net email is sponsored by the Moblin Your Move Developer's challenge > Build the coolest Linux based applications with Moblin SDK & win great prizes > Grand prize is a trip for two to an Open Source event anywhere in the world > http://moblin-contest.org/redirect.php?banner_id=100&url=/ > _______________________________________________ > phpESP-devel mailing list > php...@li... > https://lists.sourceforge.net/lists/listinfo/phpesp-devel > -- Bishop Bettini ideacode, Inc. (main) +1 919 341 5170 / (fax) +1 919 521 4100 Visit us on the web at: ideacode.com Professional software research and development reviewmysoftware.com Improve sales! Review your software before you release bytejar.com Solutions to those annoying development problems |