Not a bug Minor error reason: Registry not check key length

WeaponOwl

WeaponOwl

Active member
Affected version
2.0, 2.1
On website i work with, some time ago happens sql injections scan. Thanks to xenforo design, opposite found nothing, but it was good lesson.
This is was my fail, i used raw parameter from query as part of registry key:
\XF::registry()->get('er_time_'.$key);
Param $key received directly from url param, and it makes weak place for sql injection. Key with injection inside wasn't found in cache and tried to check database. From some moment key become so long, so provoke Data too long for column 'data_key' from mysql. So have minor bug, no limit of key length on database cache check. Happens on both 2.0 and 2.1 versions.
 
I don't think we'd consider this to be a bug, exactly.

If I'm reading this correctly, you get the Data too long error if you insert a key name which is longer than the field limit in the database (25 characters).

The registry works with what should be known keys, and not some arbitrary keys which could be generated and be too long.

If we did make some changes here, there would really only be two solutions:

1) We throw our own error, rather than relying on a database exception. There's little point in this, the result is the same.
2) We attempt to trim the key to the correct length. This is risky because it may unintentionally introduce ambiguity, e.g. 12345678901234567890123456 will be treated the same as 1234567890123456789012345 and that could cause unexpected results.

So, all in all, I don't think we'll be making any changes here.
 
The registry works with what should be known keys here the key. So it just wasn't designed for use unstatic keys. then agree, not a bug
 
Back
Top Bottom