From eb1f1a25ad44174f74db41795d39047bce7fb322 Mon Sep 17 00:00:00 2001
From: Glenn Rice <grice1@missouriwestern.edu>
Date: Tue, 18 Mar 2025 05:31:37 -0500
Subject: [PATCH 1/2] Protect static file routes from directory traversal
 attacks.

Currently using a URL like
`http://baseURL/../../../../../../../../../../../../../../../../etc/passwd`
returns the file `/etc/passwd`.  That of course is a security
vulnerability that should not happen.

This was recently discovered and fixed for webwork2, and basically the
same solution is used to fix it here.

Below is a minimal script that can be used to test this:

```perl
use Mojo::Base -strict;
use Mojo::UserAgent;

my $server = 'http://localhost:3001';

my $ua  = Mojo::UserAgent->new;
my $res = $ua->get("$server/../../../../../../../../../../../../../../../../etc/passwd")->result;

if    ($res->is_success)  { say 'success'; say $res->body }
elsif ($res->is_error)    { say 'error'; say $res->message }
elsif ($res->code == 301) { say '301'; say $res->headers->location }
else                      { say 'Whatever...' }

Mojo::IOLoop->start unless Mojo::IOLoop->is_running;
```

Set `$server` to the URL of your renderer including the `$baseURL`.

With the develop branch the above script will output "success" followed
by the contents of the `/etc/passwd` file on your server.  With this
branch it will output "error" followed by "Not found".
---
 lib/Renderer/Controller/StaticFiles.pm | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/lib/Renderer/Controller/StaticFiles.pm b/lib/Renderer/Controller/StaticFiles.pm
index 347ac626f..1130e0be6 100644
--- a/lib/Renderer/Controller/StaticFiles.pm
+++ b/lib/Renderer/Controller/StaticFiles.pm
@@ -3,9 +3,10 @@ use Mojo::Base 'Mojolicious::Controller', -signatures;
 
 use Mojo::File qw(path);
 
-sub reply_with_file_if_readable ($c, $file) {
-	if (-r $file) {
-		return $c->reply->file($file);
+sub reply_with_file_if_readable ($c, $directory, $file) {
+	my $filePath = $directory->child($file);
+	if (-r $filePath && $filePath->realpath =~ /^$directory/) {
+		return $c->reply->file($filePath);
 	} else {
 		return $c->render(data => 'File not found', status => 404);
 	}
@@ -13,23 +14,23 @@ sub reply_with_file_if_readable ($c, $file) {
 
 # Route requests for pg_files/CAPA_Graphics to render root Contrib/CAPA/CAPA_Graphics
 sub CAPA_graphics_file ($c) {
-	return $c->reply_with_file_if_readable($c->app->home->child('Contrib/CAPA/CAPA_Graphics', $c->stash('static')));
+	return $c->reply_with_file_if_readable($c->app->home->child('Contrib/CAPA/CAPA_Graphics'), $c->stash('static'));
 }
 
 # Route requests for pg_files to the render root tmp.  The
 # only requests should be for files in the temporary directory.
 # FIXME: Perhaps this directory should be configurable.
 sub temp_file ($c) {
-	$c->reply_with_file_if_readable($c->app->home->child('tmp', $c->stash('static')));
+	return $c->reply_with_file_if_readable($c->app->home->child('tmp'), $c->stash('static'));
 }
 
 # Route request to pg_files to lib/PG/htdocs.
 sub pg_file ($c) {
-	$c->reply_with_file_if_readable(path($ENV{PG_ROOT}, 'htdocs', $c->stash('static')));
+	return $c->reply_with_file_if_readable(path($ENV{PG_ROOT}, 'htdocs'), $c->stash('static'));
 }
 
 sub public_file ($c) {
-	$c->reply_with_file_if_readable($c->app->home->child('public', $c->stash('static')));
+	return $c->reply_with_file_if_readable($c->app->home->child('public'), $c->stash('static'));
 }
 
 1;

From c07716b28da1b96c7348c53b1ff8138542238fd7 Mon Sep 17 00:00:00 2001
From: Glenn Rice <grice1@missouriwestern.edu>
Date: Tue, 1 Jul 2025 06:52:31 -0500
Subject: [PATCH 2/2] Switch the static files to using a simplified
 `path_is_subdir` method from webwork2.

The problem with using `realpath` is that static assets can use symbolic
links.  In fact PG does this with static assets that reside in the same
directory as the problem file by default.  The `realpath` will not be a
subdirectory of the temporary directory in that case.  So `canonpath`
must be used instead.  It does not resolve symbolic links, or even
things like `../` in paths.  Of course the `path_is_subdir` checks those
things.
---
 lib/Renderer/Controller/StaticFiles.pm | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/lib/Renderer/Controller/StaticFiles.pm b/lib/Renderer/Controller/StaticFiles.pm
index 1130e0be6..05dae163c 100644
--- a/lib/Renderer/Controller/StaticFiles.pm
+++ b/lib/Renderer/Controller/StaticFiles.pm
@@ -1,11 +1,24 @@
 package Renderer::Controller::StaticFiles;
 use Mojo::Base 'Mojolicious::Controller', -signatures;
 
-use Mojo::File qw(path);
+use Mojo::File            qw(path);
+use File::Spec::Functions qw(canonpath);
+
+sub path_is_subdir ($path, $dir) {
+	return 0 unless $path =~ /^\//;
+
+	$path = canonpath($path);
+	return 0 if $path =~ m#(^\.\.$|^\.\./|/\.\./|/\.\.$)#;
+
+	$dir = canonpath($dir);
+	return 0 unless $path =~ m|^$dir|;
+
+	return 1;
+}
 
 sub reply_with_file_if_readable ($c, $directory, $file) {
 	my $filePath = $directory->child($file);
-	if (-r $filePath && $filePath->realpath =~ /^$directory/) {
+	if (-r $filePath && path_is_subdir($filePath, $directory)) {
 		return $c->reply->file($filePath);
 	} else {
 		return $c->render(data => 'File not found', status => 404);