[aur-dev] [PATCH 2/3] SQL: treat all UID/ID values as numbers, not strings

[Dan McGee]

On Tue, Apr 26, 2011 at 10:04 AM, Lukas Fleischer
<archlinux at cryptocrack.de> wrote:
> On Mon, Apr 25, 2011 at 11:23:01PM -0500, Dan McGee wrote:
>> Ensure we are not quoting these values in any of our SQL queries.
>>
>> Thanks-to: elij <elij.mx at gmail.com>
>> Signed-off-by: Dan McGee <dan at archlinux.org>
>> ---
>>
>> Smoke tested:
>> * Submitted a package update and it worked
>> * Updated a user's password from non-salted to salted variety
>> * Numerous login/logout cycles
>>
>>  web/html/passreset.php |    4 ++--
>>  web/html/pkgsubmit.php |    2 +-
>>  web/lib/acctfuncs.inc  |   18 +++++++++---------
>>  web/lib/aur.inc        |    6 +++---
>>  4 files changed, 15 insertions(+), 15 deletions(-)
>>
>
> Mhh. Basically looks good to me :)
>
> Did you check if all affected variables are correctly coerced into
> integers? If there's any unquoted parameter that is escaped using
> mysql_real_escape_string() instead of intval() (or something similar),
> we might become vulnerable to SQL injections, again.
>
> We should have used integer conversions in the first place, of course,
> but this patch will probably turn any mysql_real_escape_string() misuse
> into an exploitable injection vulnerability.

I did not; you may want to take a quick peek- I know you've been more
on the ball with fixing these than me, so you can probably spot the
code paths real quick. I more just made sure things worked as before.

-Dan