Opened 14 years ago

Closed 14 years ago

#13632 closed (wontfix)

lack of builtin range checking of id fields

Reported by: anonymous Owned by: None
Component: Documentation Version: dev
Severity: Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The lack of builtin range checking for id fields is a vulnerability.

/service/docserver/papers/3/ --> produces a document
/service/docserver/papers/6578/ --> produces 404 page
/service/docserver/papers/9999999999999999999/ --> throws OverflowError

Traceback is at http://paste.pocoo.org/show/218865/

I think the last case should throw DoesNotExist instead of causing server error.
In the case at hand I used generic views and sqlite3 DB backend.
Of course, one can check this himself all over the places, however that would be against the DRY principle.
Not to mention, it would be complicated when using generic views.

Change History (4)

comment:1 by Gregor Müllegger, 14 years ago

The problem with this is that the ID passed into a generic view doesn't need to be an integer. It's also reasonable that a Model has a non-integer primary key. So the key from the URL must be passed to the queryset without any type checks - we don't know the type!

You can still avoid this issue without writing custom code that checks the value before passing it into the generic view: Write the url-regexs in such a way that they force a valid type. For example:

    url(r^/service/docserver/papers/(?P<id>\d{1,6})/

This will throw a 404 for all IDs bigger than 1 000 000.

So this is generally a "wont fix" from me.

comment:2 by Yuval Adam, 14 years ago

I'm not sure this is a won't fix.

It doesn't seem reasonable to expect the user to enforce type overflow protection in her url conf.

Not to mention this is not documented...

in reply to:  2 ; comment:3 by None, 14 years ago

Component: Database layer (models, ORM)Documentation
Owner: changed from nobody to None

Replying to yuval_a:

I'm not sure this is a won't fix.

It doesn't seem reasonable to expect the user to enforce type overflow protection in her url conf.

Not to mention this is not documented...

I agree I think in the generic view documentation and maybe afew other places need to have these checks in the example.

in reply to:  3 comment:4 by None, 14 years ago

Resolution: wontfix
Status: newclosed

Replying to glassresistor:

Replying to yuval_a:

I'm not sure this is a won't fix.

It doesn't seem reasonable to expect the user to enforce type overflow protection in her url conf.

Not to mention this is not documented...

I agree I think in the generic view documentation and maybe afew other places need to have these checks in the example.

Apparently this is not something people think it is not important to have described in a tutorial. "Too complex for new users"

Note: See TracTickets for help on using tickets.
Back to Top