#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 [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/065.c4fa5f3fb760aefa0f9d2cebdb07bbba%40djangoproject.com.
