On Tue, Apr 26, 2011 at 10:44:33AM -0500, Dan McGee wrote:
On Tue, Apr 26, 2011 at 10:04 AM, Lukas Fleischer <archlinux@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@gmail.com> Signed-off-by: Dan McGee <dan@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.
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).