On Tue, Apr 26, 2011 at 10:44:33AM -0500, Dan McGee wrote:
> On Tue, Apr 26, 2011 at 10:04 AM, Lukas Fleischer
> <[email protected]> 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 <[email protected]>
> >> Signed-off-by: Dan McGee <[email protected]>
> >> ---
> >>
> >> 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.

Skimming through the affected functions, it looks like there shouldn't
be any security regression introduced by this. Pushed that (along with
the other patches this series).

Reply via email to