node.module:521 - node_load()

foreach ($param as $key => $value) {
   $cond[] = 'n.'. db_escape_string($key) ." = '%s'";
   $arguments[] = $value;
}

As $key is not used in a string context (surrouned by ''),
db_escape_string is useless. (initial report by Heine).

CommentFileSizeAuthor
#9 node_load-db_escape_table.patch1000 bytesdmitrig01
#6 node_load-db_escape_table-0.1.patch780 byteshswong3i
#1 node-escape-table.patch690 bytesajk

Comments

ajk’s picture

ajk commented
Status: Active » Needs review
StatusFileSize
new node-escape-table.patch690 bytes

And the patch.

ajk’s picture

ajk commented
Status: Needs review » Reviewed & tested by the community

As per originating issue, changing to RTBC.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Version: 6.0 » 5.x-dev

Applicable to 5.x and 7.x as well. Moving back to 5.x but also need to be committed to 7.x then.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

(I committed to 6.x.).

heine’s picture

heine commented

For the record, the initial report was by Stefan Esser, Mayflower & Zend in an audit commissioned by Die Zeit.

hswong3i’s picture

hswong3i commented
StatusFileSize
new node_load-db_escape_table-0.1.patch780 bytes

patch for D7, from http://drupal.org/node/228233

drumm’s picture

drumm
he/him
Location NY, US
commented
Version: 5.x-dev » 7.x-dev

Committed to 5.x.

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Needs work

This no longer applies, but it's going to be obsolete after #225450: Database Layer: The Next Generation gets in anyway.

dmitrig01’s picture

dmitrig01 commented
Status: Needs work » Reviewed & tested by the community
StatusFileSize
new node_load-db_escape_table.patch1000 bytes

Since it was RTBC before, this is a re-roll, and this is not a huge patch, marking RTBC again

dries’s picture

dries commented
Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks!

Anonymous’s picture

Anonymous (not verified) commented
Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.