[TYPO3-core] RFC #13940 Preventing SQL injections in CONTENT object

Martin Holtz typo3 at martinholtz.de
Fri Apr 9 00:22:12 CEST 2010


Hi Jigal,



> Version 3
> 
> Martin Holtz wrote:
>> if you define an string with comma in it:
>> your patch quotes both parts separatly:
> 
> Good catch! I followed your suggestion. Attached is version 3.
great, thanks!

i uploaded a patch with the testcases, but didnt tested yet with your third 
version. And i well, there is potential, that i did something wrong - 
especially with the NULL tests.

I think you have to remove the check for NULL values - IMHO it is not 
possible to check if a value should be NULL or "NULL".

SELECT * FROM tt_content WHERE header = "null"

is not possible than. As the "null" would be treated as NULL. And afaik in 
SQL i should use 'header IS NULL' instead of 'header = NULL' (i am not sure, 
if that would work).
So, at least the comparision to an string should be removed there. I am not 
sure, if is possible to get a NULL value there, but check for is_null() 
should not harm.

Another thing is your replacement of quotes within the comma separated list. 
So you treat "hello world" different than "hello world","hallo welt" (with 
commaSeparatedList enabled).
IMHO you should remove the parts where you remove the quotes.


And i didnt tested it yet, but i think the part
"replace the markers in the queryParts to handle stdWrap"
does not work as expected.
It should be placed right after "$queryParts = $this->getWhere($table,
$conf,TRUE);". Otherwise there are SQL-Querys where an marker, which has 
been introduced by andWhere, is not yet replaced.

I do not think i will be able to test that before monday.
And i would be really happy, if that would get into 4.4, so i will try to 
find some time.

thanks,
martin


More information about the TYPO3-team-core mailing list