Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#24625 closed Bug (fixed)

Arbitrary file inclusion in admindocs

Reported by: Markus Holtermann Owned by: Markus Holtermann
Component: contrib.admindocs Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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 "raw" or "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.

Change History (9)

comment:1 Changed 5 years ago by Markus Holtermann

Has patch: set
Needs tests: unset

comment:2 Changed 5 years ago by Markus Holtermann

Needs documentation: unset

comment:3 Changed 5 years ago by Tim Graham

Triage Stage: AcceptedReady for checkin

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?

comment:4 Changed 5 years ago by Claude Paroz

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).

comment:5 Changed 5 years ago by Markus Holtermann

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.

comment:6 Changed 5 years ago by Markus Holtermann <info@…>

Resolution: fixed
Status: newclosed

In 09595b4:

Fixed #24625 -- Prevented arbitrary file inclusion in admindocs

Thanks Tim Graham for the review.

comment:7 Changed 5 years ago by Markus Holtermann <info@…>

In 3862826:

[1.8.x] Fixed #24625 -- Prevented arbitrary file inclusion in admindocs

Thanks Tim Graham for the review.

Backport of 09595b4fc67ac4c94ed4e0d4c69acc1e4a748c81 from master

comment:8 Changed 5 years ago by Markus Holtermann <info@…>

In 3caf7efb:

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

comment:9 Changed 5 years ago by Markus Holtermann <info@…>

In 584c6591:

[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

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