#31747: Avoid potential admin model enumeration
-------------------------------+------------------------------------
Reporter: Daniel Maxson | Owner: nobody
Type: New feature | Status: new
Component: contrib.admin | Version: 3.0
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Changes (by Carlton Gibson):
* type: Bug => New feature
* stage: Unreviewed => Accepted
Comment:
Thanks Daniel.
Pasting here extracts from the security team discussion. This idea of
adding a `final_catch_all_pattern` seems most likely:
> Well, I vaguely recalled, and managed to find,
> https://groups.google.com/d/msgid/django-
developers/CAHdnYzu2zHVMcrjsSRpvRrdQBMntqy%2Bh0puWB2-uB8GOU6Tf7g%40mail.gmail.com
> where we discussed achieving similar goals using middlewares rather
> than view or URL manipulation. I believe a middleware which translates
> 404 exceptions in admin urls for unauthenticated users into redirects
> to the admin login would essentially solve the problem[1], without
> breaking existing URLConfs.
>
> If this solution of the issue is accepted, I'm not sure how to apply it
> to existing projects -- adding a middleware may require them to edit
> their settings, not just to upgrade (although the vulnerability may be
> light enough to handle by providing the middleware and just
> recommending its use).
>
> [1] Assuming we make no special effort to avoid it, this information
> may still be leaked using a timing attack -- that is, it should take a
> little more time to respond to a URL for a non-existing model. I'm not
> sure protecting against that would be worthwhile.
> > I believe a middleware which translates
> > 404 exceptions in admin urls for unauthenticated users into redirects
> > to the admin login would essentially solve the problem[1], without
> > breaking existing URLConfs.
>
> The main issues I see with a middleware is:
> a) how to enable it (as you also noticed)
> b) which URLs should it apply to?
>
> I think the latter is rather important since we explicitly advise to not
use /admin in the first place :) With an URLConf approach we could just
add a catch-all pattern to the end of admin.site.urls (with all issues
that follow from that). We might add an attribute ala
`final_catch_all_pattern = True` to `AdminSite` to allow disabling this
rather light issue (then it would be up to the site developer to add a
final catch all)
>
> FWIW this issue also applies to authenticated model enumeration
(basically a site which allows login but has limited staff/admin access --
something we should consider when coming up with the patch).
> You are correct, of course. The middleware-based approach requires
> extra configuration -- a setting, probably; and I agree that this is a
> little ugly (my thinking was a little influenced by the tags idea in
> the thread I referred to, which would mostly allow overcoming this, I
> think).
>
> > With an URLConf approach we
> > could just add a catch-all pattern to the end of admin.site.urls
> > (with all issues that follow from that). We might add an attribute
> > ala `final_catch_all_pattern = True` to `AdminSite` to allow
> > disabling this rather light issue (then it would be up to the site
> > developer to add a final catch all)
>
> That seems good enough. I'd consider, in this case, adding the option
> with default False in 3.0.x and making the non-backward-compatible
> change only in 3.1.
>
> > FWIW this issue also applies to authenticated model enumeration
> > (basically a site which allows login but has limited staff/admin
> > access -- something we should consider when coming up with the patch).
>
> IIUC, this code[1] does it, and it is arguably incorrect in the case of
> an authenticated user accessing a model they don't have permission for.
> What if we just changed that to raise a 404? Wouldn't that fix both
> cases? FWIW, that is what Github does when you try to access a private
> repo you don't have access to.
>
>
[1]https://github.com/django/django/blob/master/django/contrib/admin/sites.py#L222-L232
> > What if we just changed that to raise a 404? Wouldn't that fix both
> > cases? FWIW, that is what Github does when you try to access a private
> > repo you don't have access to.
> >
> >
[1]https://github.com/django/django/blob/master/django/contrib/admin/sites.py#L222-L232
>
> Raising a 404 would definitely help here, but the usability of that
> sucks imo.
>
> Do we think we can fix this in public and gather more input?
--
Ticket URL: <https://code.djangoproject.com/ticket/31747#comment:1>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/065.c4fa5f3fb760aefa0f9d2cebdb07bbba%40djangoproject.com.
