CVE-2022-21650: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting') (CWE-79)
value: Convos is an open source multi-user chat that runs in a web browser. You can't use SVG extension in Convos' chat window, but you can upload a file with an .html extension. By uploading an SVG file with an html extension the upload filter can be bypassed. This causes Stored XSS. Also, after uploading a file the XSS attack is triggered upon a user viewing the file. Through this vulnerability, an attacker is capable to execute malicious scripts. Users are advised to update as soon as possible.
CWE description: The product does not neutralize or incorrectly neutralizes user-controllable input before it is placed in output that is used as a web page that is served to other users.
CVSS v3 base score: 5.4
Published: 2022-01-04T21:15Z
Repository: https://github.com/convos-chat/convos
Language: Perl
Files changed: Changes, lib/Convos/Controller/Files.pm, lib/Convos/Core/User/File.pm, t/data/markup.html, t/data/markup.xhtml, t/web-files.t
Commit date: 2021-12-29 15:30:34 +0900
Commit message: Add more rules for files with Content-Disposition

Diff: diff --git a/Changes b/Changes
index 590a5b02..d45ed09d 100644
--- a/Changes
+++ b/Changes
@@ -1,9 +1,12 @@
 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
    Contributor: KhanhCM
    Reference: https://huntr.dev/bounties/142c3dcc-a882-4799-949b-520be0a4d144
diff --git a/lib/Convos/Controller/Files.pm b/lib/Convos/Controller/Files.pm
index c11377f9..42590cbd 100644
--- a/lib/Convos/Controller/Files.pm
+++ b/lib/Convos/Controller/Files.pm
@@ -2,16 +2,18 @@ package Convos::Controller::Files;
 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 {
     return $self->reply->not_found unless eval { $file->filename };    # invalid fid
     return $self->reply->not_found if $file->write_only;
@@ -23,11 +25,11 @@ sub get {
     my $format = $self->stash('format') || '';
     return $self->render(file => file => $file) if !$format and $ct =~ m!$type_can_be_embedded!;
 
     $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);
   });
 }
 
 sub list {
diff --git a/lib/Convos/Core/User/File.pm b/lib/Convos/Core/User/File.pm
index d823ecb0..5c947ea1 100644
--- a/lib/Convos/Core/User/File.pm
+++ b/lib/Convos/Core/User/File.pm
@@ -26,13 +26,13 @@ has path => sub {
   my @uri  = split '/', $self->uri;
   $uri[-1] =~ s!\.json$!.data!;
   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 {
   my ($class, $backend, $connection, $message) = @_;
   my $self = $class->new(user => $connection->user);
diff --git a/t/data/markup.html b/t/data/markup.html
new file mode 100644
index 00000000..69e9da41
--- /dev/null
+++ 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
new file mode 100644
index 00000000..e3d5bac9
--- /dev/null
+++ 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
index 83f3e46d..6b9ed0b2 100644
--- a/t/web-files.t
+++ b/t/web-files.t
@@ -84,18 +84,41 @@ subtest 'embedded image' => sub {
 
   $t->get_ok("/file/1/$fid.jpg")->header_is('Cache-Control', 'max-age=86400')
     ->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 {
   note 'with-script-tag.svg';
   $t->post_ok('/api/files', form => {file => {file => 't/data/with-script-tag.svg'}})
@@ -141,11 +164,11 @@ subtest 'list' => sub {
   my $files = sub { @{$t->tx->res->json->{files}} };
   $t->get_ok('/api/files')->status_is(200)->json_hasnt('/after')->json_hasnt('/next')
     ->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', [])
     ->json_hasnt('/after')->json_hasnt('/next')->json_hasnt('/prev');
 

Vulnerable code:   my $file = $self->_file(id => $self->stash('fid'), user => $user, types => $self->app->types);
  state $type_can_be_embedded = qr{^(application/javascript|application/json|image|text)};
      unless $ct =~ m!$type_can_be_embedded!;
has saved      => sub { Mojo::Date->new->to_datetime };
has types      => sub { Mojolicious::Types->new };
has user       => undef;
subtest 'binary' => sub {
  is @ids, 14, 'got all files';
Fixed code:  - Add more rules for files with Content-Disposition
   Contributor: Pocas
   Reference: https://huntr.dev/bounties/ae424798-de01-4972-b73b-2db674f82368/
  my $file = $self->_file(id => $self->stash('fid'), user => $user);
  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/)};
      unless $ct =~ m!$type_can_be_viewed!;
has saved => sub { Mojo::Date->new->to_datetime };
has types => sub { state $types = Mojolicious::Types->new->type(xhtml => 'application/xhtml+xml') };
has user  => undef;
<!DOCTYPE html>
<html></html>
<!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>
subtest 'attachment' => sub {
  note 'binary';

  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';
  is @ids, 16, 'got all files';