Issue G: Unescaped Characters Put Into Content-Disposition Header #832

Closed
fpietrosanti opened this Issue Mar 4, 2014 · 3 comments

Projects

None yet

3 participants

@fpietrosanti
Contributor

Synopsis: When the whistleblower uploads a file, they provide its file name. That file name is stored in
the GlobaLeaks database. When the receiver downloads the file, the name provided will be reflected nto the HTTP headers that are sent to the receiver, without being escaped.

@fpietrosanti fpietrosanti added this to the LeastAuthorityPentest milestone Mar 4, 2014
@evilaliv3 evilaliv3 added a commit to globaleaks/GLBackend-outdated that referenced this issue Apr 9, 2014
@evilaliv3 evilaliv3 addressed issue globaleaks/GlobaLeaks#832 3504afe
@evilaliv3
Contributor

@defuse:

in order to address this issue i've applied the following changes:

  • we do not rely anymore on custom regexp in order to validate Content-Disposition header
  • we instead rely on standard cgi.parse_header

related to the http split injection i've further investigated and it does not sussist. in fact the set_header function of cyclone calls the _convert_header_value that prevent usage of the following values [\x00-\x1f].

in addition as suggested i've corretly url encoded/decoded the filename as suggested in the report by using the standard urllib.encode/decode.

can you please validate the solution?

@defuse
defuse commented Apr 9, 2014

@evilaliv3 I believe the security issue has been fixed, but the actual setting of the Content-Disposition header for the download might be incorrect for some browsers: See here.

The simplest 'correct' way to do it is to put the filename in the URL of the download link or button, then don't set the filename with Content-Disposition, since the browser will automatically use the filename from the URL. So the URL would be something like this:

http://uzekbw3injzwsox2.onion/rtip/57c5cf27-b993-4a82-b3ca-98f55b890df2/download/9c41be2a-e319-48be-ab74-627093a6cca9/the-filename-goes-here.pdf

Then have the server just ignore the last the-filename-goes-here.pdf part, since its only purpose is to be the filename the browser will use.

@evilaliv3
Contributor

allright, we will evaluate this possibility thank you.

@evilaliv3 evilaliv3 added a commit to globaleaks/GLBackend-outdated that referenced this issue Apr 11, 2014
@evilaliv3 @hellais evilaliv3 + hellais addressed issue globaleaks/GlobaLeaks#832 d8b4f2c
@evilaliv3 evilaliv3 added a commit to globaleaks/GLBackend-outdated that referenced this issue Apr 11, 2014
@evilaliv3 @hellais evilaliv3 + hellais addressed issue globaleaks/GlobaLeaks#832 7034813
@evilaliv3 evilaliv3 added a commit to globaleaks/GLBackend-outdated that referenced this issue Apr 11, 2014
@evilaliv3 @hellais evilaliv3 + hellais addressed issue globaleaks/GlobaLeaks#832 dc362fe
@evilaliv3 evilaliv3 closed this Apr 14, 2014
@evilaliv3 evilaliv3 changed the title from Issue G. Confidential to Issue G: Unescaped Characters Put Into Content-Disposition Header Apr 20, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment