[Django] #24625: Arbitrary file inclusion in admindocs
Django
---------------------------------------------+-------------------------
Reporter: MarkusH | Owner: MarkusH
Type: Bug | Status: new
Component: contrib.admindocs | Version: master
Severity: Normal | Keywords:
Triage Stage: Accepted | Has patch: 0
Needs documentation: 1 | Needs tests: 1
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
---------------------------------------------+-------------------------
After consulting with the security team we're treating this issue as a
hardening:
`django.contrib.admindocs` relies on Docutils to render the docstrings.
Docutils has the two directives
"[http://docutils.sourceforge.net/docs/ref/rst/directives.html#raw-
directive raw]" or
"[http://docutils.sourceforge.net/docs/ref/rst/directives.html#include
include]" to include files. By installing a 3rd party app and not
carefully reviewing model, view or other docstrings, an attacker can
insert arbitrary HTML code posing as a XSS vulnerability as well as
include arbitrary files, e.g. the Django project settings, potentially
revealing the database password and secret key.
--
Ticket URL: <https://code.djangoproject.com/ticket/24625>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: MarkusH | Owner: MarkusH
Type: Bug | Status: new
Component: contrib.admindocs | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by MarkusH):
* has_patch: 0 => 1
* needs_tests: 1 => 0
Comment:
PR: https://github.com/django/django/pull/4478
--
Ticket URL: <https://code.djangoproject.com/ticket/24625#comment:1>
Django
Reporter: MarkusH | Owner: MarkusH
Type: Bug | Status: new
Component: contrib.admindocs | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by MarkusH):
* needs_docs: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/24625#comment:2>
Django
Reporter: MarkusH | Owner: MarkusH
Type: Bug | Status: new
Component: contrib.admindocs | Version: master
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* stage: Accepted => Ready for checkin
Comment:
I don't really see the issue as needing a backport based on our current
policy, but I don't mind other than the perception that the core team
plays by different rules. Do you want to lobby for adding "security
hardening measures" to the list of fixes that will be backported?
--
Ticket URL: <https://code.djangoproject.com/ticket/24625#comment:3>
Django
Reporter: MarkusH | Owner: MarkusH
Type: Bug | Status: new
Component: contrib.admindocs | Version: master
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by claudep):
I think that when a core dev is volunteering to backport a patch, he
should be allowed to do so (unless it obviously adds some regression
risks).
--
Ticket URL: <https://code.djangoproject.com/ticket/24625#comment:4>
Django
Reporter: MarkusH | Owner: MarkusH
Type: Bug | Status: new
Component: contrib.admindocs | Version: master
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by MarkusH):
I think it mostly depends on the size and impact of the patch. The one for
this issue is rather trivial and users who run into a regression with it
will have other problems than outlined in this issue. Since it's not a
security issue per se I'm fine with not backporting it to anything. It
just feels dump to keep that issue in a release that we'll support for 3+
years which is why I'm +1 on backporting it to 1.8. Those users who are on
1.7 are likely going to update to 1.8 sooner or later and are fine then.
--
Ticket URL: <https://code.djangoproject.com/ticket/24625#comment:5>
Django
Reporter: MarkusH | Owner: MarkusH
Type: Bug | Status: closed
Component: contrib.admindocs | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Markus Holtermann <info@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"09595b4fc67ac4c94ed4e0d4c69acc1e4a748c81" 09595b4]:
{{{
#!CommitTicketReference repository=""
revision="09595b4fc67ac4c94ed4e0d4c69acc1e4a748c81"
Fixed #24625 -- Prevented arbitrary file inclusion in admindocs
Thanks Tim Graham for the review.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24625#comment:6>
Django
Reporter: MarkusH | Owner: MarkusH
Type: Bug | Status: closed
Component: contrib.admindocs | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Markus Holtermann <info@…>):
In [changeset:"3862826fedc99378279b85e602079b53593ae129" 3862826]:
{{{
#!CommitTicketReference repository=""
revision="3862826fedc99378279b85e602079b53593ae129"
[1.8.x] Fixed #24625 -- Prevented arbitrary file inclusion in admindocs
Thanks Tim Graham for the review.
Backport of 09595b4fc67ac4c94ed4e0d4c69acc1e4a748c81 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24625#comment:7>
Django
Reporter: MarkusH | Owner: MarkusH
Type: Bug | Status: closed
Component: contrib.admindocs | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Markus Holtermann <info@…>):
In [changeset:"3caf7efb44712f89d6552076c240a3c898673a2c" 3caf7efb]:
{{{
#!CommitTicketReference repository=""
revision="3caf7efb44712f89d6552076c240a3c898673a2c"
Refs #24625 -- Filtered docutils warnings output in tests
Instead of setting ``warning_stream`` in the docutils config overrides
to ``False`` I opted for filtering the stderr in the tests to keep the
error output showing up in server logs.
Thanks Tim Graham for the report and review
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24625#comment:8>
Django
Reporter: MarkusH | Owner: MarkusH
Type: Bug | Status: closed
Component: contrib.admindocs | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Markus Holtermann <info@…>):
In [changeset:"584c6591a3c29c94026e3bebc3e5302a3d7530e3" 584c6591]:
{{{
#!CommitTicketReference repository=""
revision="584c6591a3c29c94026e3bebc3e5302a3d7530e3"
[1.8.x] Refs #24625 -- Filtered docutils warnings output in tests
Instead of setting ``warning_stream`` in the docutils config overrides
to ``False`` I opted for filtering the stderr in the tests to keep the
error output showing up in server logs.
Thanks Tim Graham for the report and review
Backport of 3caf7efb44712f89d6552076c240a3c898673a2c from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24625#comment:9>