Find files that are affected by APPSEC-1063

[arosenhagen]

Find files that are affected by APPSEC-1063, addressing possible SQL injection with command "dev:possible-sql-injection"

[peterjaap]

Great stuff, thanks!! :)

[hostep]

Tuning in to mention that yesterday we discovered an extension which does something like this which also breaks due to APPSEC-1063:

return parent::addFieldToFilter("IF($field.value IS NULL, main_table.$field, $field.value)", $condition);

So you should probably update your checks to include the phrase addFieldToFilter("IF(. Or maybe just check if a ( character is somewhere in between the quotes and not just as a first character?

Also watch out for double vs single quotes, you are now only checking for single quotes.

[arosenhagen]

so maybe the following will do it?

/addFieldToFilter[\n\r\s]*\([\n\r\s]*[\'"]?[\`]?.*[\(]?/i

effectively searching for ( inside of addFieldToFilter(

-- update -- small edit to

/addFieldToFilter[\n\r\s]*\([\n\r\s]*[\'"].*[\`\(]/i

but anyways this will generate false positives. I fear this case must be caught manually

[hostep]

I agree, there are too many false positives when trying to do something like that. Maybe we could try something like Tokenization to get the first param from the addFieldToFilter call instead of regex checking, but that's probably a lot of work.

My take on the regex:

/addFieldToFilter\s*\(\s*[\'"](`|\(|IF\()/i

I removed the \n & \r since they are already included in \s and added a check for the IF(, but I can image if you do a COUNT( or MAX( or whatever, it will also break against APPSEC-1063

[arosenhagen]

just a bit OT: I stumbled upon the exact same issue you have (Magestore_Bannerslider). Since there is no official update of the extension - can you give a hint, how you solved the addFieldToFilter("IF(issue?

[hostep]

Sure, we had it with the Magestore_Storepickup extension, looks like they are reusing a lot of their code :)

             }
-            return parent::addFieldToFilter("IF($field.value IS NULL, main_table.$field, $field.value)", $condition);
+            $this->getSelect()->where("IF($field.value IS NULL, main_table.$field, $field.value) = $condition");
+            return $this;
         }

Or you can ask their support, they reply pretty quick (less then a day) and they send me a patch to fix it.

[arosenhagen]

thanks! this works (linked this thread in Extensions that will break with SUPEE-6788 / Magento 1.9.2.2 / EE 1.14.2.2)