Re: [Django] #25905: Unsafe usage of urljoin() within FileStorageSystem

[Django]

#25905: Unsafe usage of urljoin() within FileStorageSystem
-+-
 Reporter:  Aman Ali |Owner:  Tobias
 Type:   |  Kunze
  Cleanup/optimization   |   Status:  closed
Component:  File |  Version:  1.9
  uploads/storage|
 Severity:  Normal   |   Resolution:  fixed
 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
-+-

Comment (by Tobias Kunze):

 Thank you for your feedback – as this ticket has been closed some time
 ago, the best way forward would be to open a new ticket. There you could
 describe the behaviour you would like to see as opposed to the current
 one, and reference this ticket as introducing current behaviour. A new
 ticket will run through the proper ticket lifecycle and (more importantly)
 will get the attention of other developers.

-- 
Ticket URL: 
Django 
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/069.2cd868f69ee4437835fc292524b84f87%40djangoproject.com.

Re: [Django] #25905: Unsafe usage of urljoin() within FileStorageSystem

[Django]

#25905: Unsafe usage of urljoin() within FileStorageSystem
-+-
 Reporter:  Aman Ali |Owner:  Tobias
 Type:   |  Kunze
  Cleanup/optimization   |   Status:  closed
Component:  File |  Version:  1.9
  uploads/storage|
 Severity:  Normal   |   Resolution:  fixed
 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
-+-
Changes (by bhch):

 * cc: bhch (added)


Comment:

 The
 
[https://github.com/django/django/commit/fdf5cd3429369954e8deb764d9f30f6374581613
 #diff-87c0869f58253f571c08ccf0fc5c7465R410 current fix] strips off all the
 leading slashes thereby making it a relative path, to which `urljoin`
 later prepends a base url.  This removes the possibility of serving a
 default file from the static url.

 I think a better solution would be to remove more than 1 leading slashes,
 but not one.

 Current implementation: `url.lstrip('/')`.

 Proposed: `re.sub(r'/{2,}', '/', url)`.

 This will allow us to serve a default file from static url.

-- 
Ticket URL: 
Django 
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/069.717ca33680157371c9949cf495119728%40djangoproject.com.

Re: [Django] #25905: Unsafe usage of urljoin() within FileStorageSystem

[Django]

#25905: Unsafe usage of urljoin() within FileStorageSystem
--+
 Reporter:  free-Runner   |Owner:  rixx
 Type:  Cleanup/optimization  |   Status:  closed
Component:  File uploads/storage  |  Version:  1.9
 Severity:  Normal|   Resolution:  fixed
 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
--+
Changes (by Russell Keith-Magee ):

 * status:  assigned => closed
 * resolution:   => fixed


Comment:

 In [changeset:"fdf5cd3429369954e8deb764d9f30f6374581613" fdf5cd34]:
 {{{
 #!CommitTicketReference repository=""
 revision="fdf5cd3429369954e8deb764d9f30f6374581613"
 Fixed #25905 -- Prevented leading slashes in urljoin() calls

 Leading slashes in the second urljoin argument will return exactly that

 argument, breaking FileSystemStorage.url behavior if called with a

 parameter with leading slashes.

 Also added test cases for null bytes and None. Thanks to Markus for

 help and review.
 }}}

--
Ticket URL: 
Django 
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.ce798beaf391fa2bbd7ca04559fffabb%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #25905: Unsafe usage of urljoin() within FileStorageSystem

[Django]

#25905: Unsafe usage of urljoin() within FileStorageSystem
--+
 Reporter:  free-Runner   |Owner:  rixx
 Type:  Cleanup/optimization  |   Status:  assigned
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
--+
Changes (by rixx):

 * owner:  nobody => rixx
 * status:  new => assigned
 * needs_tests:  1 => 0


--
Ticket URL: 
Django 
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.7bbfaf5e858a9e53a20d16df29a89ca5%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #25905: Unsafe usage of urljoin() within FileStorageSystem

[Django]

#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:  1 |  Patch needs improvement:  0
Easy pickings:  0 |UI/UX:  0
--+
Changes (by timgraham):

 * needs_tests:  0 => 1


Comment:

 A test is also needed. If you could send a pull request after adding that,
 it's easiest for review purposes.

--
Ticket URL: 
Django 
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.b690bf50de12dd7f97e370d05a9a253b%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #25905: Unsafe usage of urljoin() within FileStorageSystem

[Django]

#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: 
Django 
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 

Re: [Django] #25905: Unsafe usage of urljoin() within FileStorageSystem

[Django]

#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
--+
Changes (by timgraham):

 * type:  Bug => Cleanup/optimization
 * component:  Core (Other) => File uploads/storage
 * stage:  Unreviewed => Accepted


--
Ticket URL: 
Django 
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.5ea5e3972af915489574f2c5c13d5542%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #25905: Unsafe usage of urljoin() within FileStorageSystem

[Django]

#25905: Unsafe usage of urljoin() within FileStorageSystem
---+--
 Reporter:  free-Runner|Owner:  nobody
 Type:  Bug|   Status:  new
Component:  Core (Other)   |  Version:  1.9
 Severity:  Normal |   Resolution:
 Keywords:  file, storage  | Triage Stage:  Unreviewed
Has patch:  1  |  Needs documentation:  0
  Needs tests:  0  |  Patch needs improvement:  0
Easy pickings:  0  |UI/UX:  0
---+--
Changes (by free-Runner):

 * cc: aman.ali@… (added)
 * needs_better_patch:   => 0
 * needs_tests:   => 0
 * needs_docs:   => 0


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

--

--
Ticket URL: 
Django 
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 

[Django] #25905: Unsafe usage of urljoin() within FileStorageSystem

[Django]

#25905: Unsafe usage of urljoin() within FileStorageSystem
--+---
 Reporter:  free-Runner   |  Owner:  nobody
 Type:  Bug   | Status:  new
Component:  Core (Other)  |Version:  1.9
 Severity:  Normal|   Keywords:  file, storage
 Triage Stage:  Unreviewed|  Has patch:  1
Easy pickings:  0 |  UI/UX:  0
--+---
 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 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: 
Django 
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/054.d5d9eed55f8fff392288942235b8%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.