Security risk/exception in Will_paginate + Plus Fix
Reported by Andrew Nesbitt | October 25th, 2007 @ 06:41 PM
Hey
I notice that a spam bot hit my site which uses will paginate and caused a nasty exception:
Mysql::Error: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '-10, 10' at line 1: SELECT * FROM tickets WHERE (private = false) ORDER BY created_at DESC LIMIT -10, 10
It tried to put some escaped html into the page params with this url:
http://ticketapp.com/tickets?pag...
So I've tweaked line 19 of collection.rb to make sure params[:page] is a number, here is the file in a pastie: http://pastie.caboo.se/110837
I hope this is helpful (my first patch ever), feel free to refactor as you see fit.
Thanks
Andrew
Comments and changes to this ticket
-
Mislav October 25th, 2007 @ 06:41 PM
- State changed from new to invalid
Hey Andrew. First of all, thanks for the report and proposed fix. I understand your concern, but this isn't a security risk.
Because we are using ActiveRecord limit/offset there is no way that an attacker could inject arbitrary SQL in the queries. He can try to pass text or HTML like this, but the effort will be futile since it would result in an error. I don't want to silence errors here by sanitizing input - these errors should happen when you give your controllers nonsense input. It's up to you to deal with those errors gracefully (eg. show a nice error page).
So, I'm closing this. If you stumble upon anything other that also might be a security hole, don't hesitate to report it again. I will review it as I did now and act accordingly.
Please Sign in or create a free account to add a new ticket.
With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile ยป
Everyone's favorite Ruby library for pagination of practically anything!
People watching this ticket
Tags
Referenced by
- 129 will_paginate should make sure that the page param is an integer http://err.lighthouseapp.com/pro...
- 217 Will paginate causes exception when given non-numerical :page It's supposed to throw an error so you can handle it as ...