Skip to content

Commit 33ed106

Browse files
committed
security: HTML File Browser Execution (Windows: Firefox/IE)
This addresses an issue reported by Aishwarya Iyer where attached HTML files are executed in the browser instead of forcing download in Firefox and IE for Windows specifically. This is caused by an incorrect `Content-Disposition` set in the `AttachmentFile::download` function. Instead of attachments having a disposition of `attachment` (which forces download) they have a disposition of `inline` (which displays the file contents in the browser). This updates the download function to use whatever disposition is passed (for S3 plugin), if none it defaults to `attachment`. In addition, this overwrites the disposition and sets it to `attachment` after the `$bk->sendRedirectURL()` so that S3 attachments still work and the issue of an attacker passing their own disposition is mitigated.
1 parent 9e5a476 commit 33ed106

File tree

1 file changed

+4
-2
lines changed

1 file changed

+4
-2
lines changed

Diff for: include/class.file.php

+4-2
Original file line numberDiff line numberDiff line change
@@ -240,14 +240,16 @@ static function _genUrlSignature($id, $key, $signature, $expires) {
240240
}
241241

242242
function download($disposition=false, $expires=false) {
243-
$disposition = $disposition ?: 'inline';
243+
$disposition = ($disposition && strcasecmp($disposition, 'inline') == 0
244+
&& strpos($this->getType(), 'image/') !== false)
245+
? 'inline' : 'attachment';
244246
$bk = $this->open();
245247
if ($bk->sendRedirectUrl($disposition))
246248
return;
247249
$ttl = ($expires) ? $expires - Misc::gmtime() : false;
248250
$this->makeCacheable($ttl);
249251
$type = $this->getType() ?: 'application/octet-stream';
250-
Http::download($this->getName(), $type, null, 'inline');
252+
Http::download($this->getName(), $type, null, $disposition);
251253
header('Content-Length: '.$this->getSize());
252254
$this->sendData(false);
253255
exit();

0 commit comments

Comments
 (0)