Easy way to crash your db server

[dipth]

There doesn't seem to be any sane restrictions on what page number you can request, so if you just go to any page that uses this library for pagination and change the page-querystring to something like 99999999, Kerosene will fire off an insanely heavy db query. Do this enough times and you have a super easy way to DDOS the site...

[dipth]

I also noticed that whatever page-number you put in, will result in that many page-links being rendered by the paginate helper. For instance if you put in 1000 as the page number, the helper will render 1000 page-links even though there might only be 1 page.

Might I suggest the following changes:

• Make sure that there is some sane restriction on the upper bound of what page number will be accepted.
• Don't render page links for pages that are above the actual total_pages
• Don't render page links for each and every page, but instead only render some N-number of page links around the current page:

« First ‹ Prev ... 2 3 4 [5] 6 7 8 ... Next › Last »

[allyraza]

I see this is potentially a problem for any pagination library, given the fact we don't know how many pages 1 might have in his/her database, I simply can't put limit to that in library that might cause other issue on the application level, let me think about it how can we solve this in kerosene. as far the second one helper is restricted I believe if I am not mistaken will double check it.

do you have any ideas?

[dipth]

Personally I would be ok with the library first doing a count on all the records to calculate a max page number

[merqlove]

Yeah, i'm too think better is to have max_page. Look up to https://github.com/kaminari/kaminari.

[allyraza]

agree max_page option would be nice

[OkayItsMikael]

I made a PR to https://github.com/mgwidmann/scrivener_html to fix this issue, take a look and see if it helps: https://github.com/mgwidmann/scrivener_html/pull/52/files

[allyraza]

I have pushed a patch for this, if the page is greater then total_pages (item count / per_page) the we use total_pages as current page

please give it a spin if it works as expected

[allyraza]

@dipth Indeed you are right the pages are being generated for total_pages which is weird the windows are not being enforced, I accepted a PR sometime ago I think that may have messed up the html generator will double check it. thanks