#25905: Unsafe usage of urljoin() within FileStorageSystem
--------------------------------------+------------------------------------
Reporter: free-Runner | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: File uploads/storage | Version: 1.9
Severity: Normal | Resolution:
Keywords: file, storage | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Description changed by free-Runner:
Old description:
> It may be possible to override the hostname when displaying a link to a
> static/media file with an attacker-controlled filename. The url() method
> in FileStorageSystem uses urljoin(base,url) to combine the base_url
> (typically STATIC_URL or MEDIA_URL) to the filename. The urljoin function
> has an edge case where if the url parameter starts with "//", the
> base_url value is overwritten. Thus, if an attacker can set the name
> variable of a FileStorageSystem instance to "//www.evil.com", any url
> method call on that instance will return an external link pointing to the
> attacker's site. Creating a file of the name "\\www.evil.com" is also
> acceptable as the filepath_to_uri function (that is called on the
> filename before the urljoin call) converts all backslashes to forward-
> slashes. The latter example works better as it is a completely valid
> Linux file name.
>
> This issue can't be exploited using framework-provided upload techniques
> (FileFields, ImageFields, etc) as they properly escape the filename.
> However, an application may directly initialize a FileStorageSystem using
> user-controlled data or allow users to modify the name attribute of an
> existing FileStorageSystem instance. In such cases, an attacker could
> convert the expected relative paths into absolute external URLs.
>
> This issue can simply be patched by modifying the line found
> here[https://github.com/django/django/blob/master/django/core/files/storage.py#L302]
> as follows:
>
> return urljoin(self.base_url,
> filepath_to_uri(get_valid_filename(name)))
>
> This change filters the filename provided through the get_valid_filename
> function from django.utils.text. This function does a sufficient job of
> eliminating the ability to override the base_url.
>
> Note: This issue was initially disclosed to the Django security team and
> was decided not to be treated as a security issue, but instead a bug.
New description:
It may be possible to override the hostname when displaying a link to a
static/media file with an attacker-controlled filename. The url() method
in FileStorageSystem uses urljoin(base,url) to combine the base_url
(typically STATIC_URL or MEDIA_URL) to the filename. The urljoin function
has an edge case where if the url parameter starts with {{{"//"}}}, the
base_url value is overwritten. Thus, if an attacker can set the name
variable of a FileStorageSystem instance to {{{"//www.evil.com"}}}, any
url method call on that instance will return an external link pointing to
the attacker's site. Creating a file of the name {{{"\\www.evil.com"}}} is
also acceptable as the filepath_to_uri function (that is called on the
filename before the urljoin call) converts all backslashes to forward-
slashes. The latter example works better as it is a completely valid Linux
file name.
This issue can't be exploited using framework-provided upload techniques
(FileFields, ImageFields, etc) as they properly escape the filename.
However, an application may directly initialize a FileStorageSystem using
user-controlled data or allow users to modify the name attribute of an
existing FileStorageSystem instance. In such cases, an attacker could
convert the expected relative paths into absolute external URLs.
This issue can simply be patched by modifying the line found
here[https://github.com/django/django/blob/master/django/core/files/storage.py#L302]
as follows:
{{{return urljoin(self.base_url,
filepath_to_uri(name).replace('//','/'))}}}
This change filters the filename provided through the get_valid_filename
function from django.utils.text. This function does a sufficient job of
eliminating the ability to override the base_url.
Note: This issue was initially disclosed to the Django security team and
was decided not to be treated as a security issue, but instead a bug.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/25905#comment:3>
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 post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/069.8c31ecb8485e6ee2a911b416ee561a16%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
