Add more rules for files with Content-Disposition

convos-chat · Dec 29, 2021 · 5c0a1ec · 5c0a1ec
1 parent 9645ce5
commit 5c0a1ec
Show file tree

Hide file tree

Showing 6 changed files with 40 additions and 8 deletions.
diff --git a/Changes b/Changes
@@ -2,6 +2,9 @@ Revision history for perl distribution Convos
 
 6.48 Not Released
  - Fix catching invalid nick change #628
+ - Add more rules for files with Content-Disposition
+   Contributor: Pocas
+   Reference: https://huntr.dev/bounties/ae424798-de01-4972-b73b-2db674f82368/
 
 6.47 2021-12-28T12:32:00+0900
  - Add more rules for potentially evil svg files

diff --git a/lib/Convos/Controller/Files.pm b/lib/Convos/Controller/Files.pm
@@ -4,12 +4,14 @@ use Mojo::Base 'Mojolicious::Controller';
 sub get {
   my $self = shift;
   my $user = $self->app->core->get_user_by_uid($self->stash('uid'));
-  my $file = $self->_file(id => $self->stash('fid'), user => $user, types => $self->app->types);
+  my $file = $self->_file(id => $self->stash('fid'), user => $user);
 
   # Make sure we don't get 501 Not implemented if this is an API request
   $self->stash(handler => 'ep', openapi => 'Should never be rendered.');
 
-  state $type_can_be_embedded = qr{^(application/javascript|application/json|image|text)};
+  state $type_can_be_embedded
+    = qr{^(application/javascript|application/(json|xhtml|xml)|image|text)};
+  state $type_can_be_viewed = qr{^(application/javascript|audio/|image/|text/plain|video/)};
 
   return $self->reply->not_found unless $file->user;    # invalid uid
   return $file->load_p->then(sub {
@@ -25,7 +27,7 @@ sub get {
 
     $h->content_type($ct);
     $h->content_disposition(qq[attachment; filename="@{[$file->filename]}"])
-      unless $ct =~ m!$type_can_be_embedded!;
+      unless $ct =~ m!$type_can_be_viewed!;
     return $self->reply->asset($file->asset);
   });
 }

diff --git a/lib/Convos/Core/User/File.pm b/lib/Convos/Core/User/File.pm
@@ -28,9 +28,9 @@ has path => sub {
   return $self->user->core->home->child(@uri);
 };
 
-has saved      => sub { Mojo::Date->new->to_datetime };
-has types      => sub { Mojolicious::Types->new };
-has user       => undef;
+has saved => sub { Mojo::Date->new->to_datetime };
+has types => sub { state $types = Mojolicious::Types->new->type(xhtml => 'application/xhtml+xml') };
+has user  => undef;
 has write_only => sub {false};
 
 sub handle_message_to_paste_p {

diff --git a/t/data/markup.html b/t/data/markup.html
@@ -0,0 +1,2 @@
+<!DOCTYPE html>
+<html></html>
diff --git a/t/data/markup.xhtml b/t/data/markup.xhtml
@@ -0,0 +1,2 @@
+<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
+<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en"></html>
diff --git a/t/web-files.t b/t/web-files.t
@@ -86,14 +86,37 @@ subtest 'embedded image' => sub {
     ->header_is('Content-Type' => 'image/jpeg')->header_exists_not('Content-Disposition');
 };
 
-subtest 'binary' => sub {
+subtest 'attachment' => sub {
+  note 'binary';
   $t->post_ok('/api/files', form => {file => {file => 't/data/binary.bin'}})->status_is(200);
   my $fid  = $t->tx->res->json('/files/0/id');
   my $url  = $t->tx->res->json('/files/0/url');
   my $name = $t->tx->res->json('/files/0/filename');
   $t->get_ok("/file/1/$fid")->header_is('Cache-Control', 'max-age=86400')
     ->header_is('Content-Disposition', 'attachment; filename="binary.bin"')
     ->header_is('Content-Type' => 'application/octet-stream');
+
+  note 'html';
+  $t->post_ok('/api/files', form => {file => {file => 't/data/markup.html'}})->status_is(200);
+  $fid = $t->tx->res->json('/files/0/id');
+  $t->get_ok("/file/1/$fid.html")
+    ->header_is('Content-Disposition', 'attachment; filename="markup.html"')
+    ->header_is('Content-Type' => 'text/html;charset=UTF-8')->content_like(qr{^<!DOCTYPE html>});
+  $t->get_ok("/file/1/$fid")->header_is('Content-Disposition', undef)
+    ->header_is('Content-Type' => 'text/html;charset=UTF-8')->text_is('h1', 'markup.html');
+  like $t->tx->res->dom->at('div.le-paste'), qr{&lt;!DOCTYPE}, 'embedded and escaped html';
+
+  note 'xhtml';
+  $t->post_ok('/api/files', form => {file => {file => 't/data/markup.xhtml'}})->status_is(200);
+  $fid = $t->tx->res->json('/files/0/id');
+  $t->get_ok("/file/1/$fid.xhtml")
+    ->header_is('Content-Disposition', 'attachment; filename="markup.xhtml"')
+    ->header_is('Content-Type' => 'application/xhtml+xml')
+    ->content_like(qr{^<!DOCTYPE html PUBLIC});
+  $t->get_ok("/file/1/$fid")->header_is('Content-Disposition', undef)
+    ->header_is('Content-Type' => 'text/html;charset=UTF-8')->text_is('h1', 'markup.xhtml');
+  like $t->tx->res->dom->at('div.le-paste'), qr{&lt;!DOCTYPE html PUBLIC},
+    'embedded and escaped xhtml';
 };
 
 subtest 'svg with javascript' => sub {
@@ -143,7 +166,7 @@ subtest 'list' => sub {
     ->json_hasnt('/prev')->json_has('/files/4')->json_has('/files/0/name')->json_has('/files/0/id')
     ->json_like('/files/0/saved', qr{^\d+-\d+-\d+T})->json_has('/files/0/size');
   my @ids = map { $_->{id} } $files->();
-  is @ids, 14, 'got all files';
+  is @ids, 16, 'got all files';
 
   note 'unknown';
   $t->get_ok('/api/files?limit=1&after=unknown')->status_is(200)->json_is('/files', [])