Hi.
Here is a patch series for fateserver, to fix warnings and enable Perl's
taint checks, thus protecting against a whole class of security issues.
There would be more work to make this really clean, but I need to wait
for my ISP to fix its crap before I can do it comfortably. If it looks
to work correctly on the real or beta server and it is confirmed that
the recent issue is avoided, please feel free to apply without waiting
for me.
Regards,
--
Nicolas George
From 14230b19cd3b194888a070905a8b7301d9f8980f Mon Sep 17 00:00:00 2001
From: Nicolas George <geo...@nsup.org>
Date: Sun, 8 Aug 2021 15:12:17 +0200
Subject: [PATCH 01/11] FATE: cosmetic: reorder @EXPORT.
Signed-off-by: Nicolas George <geo...@nsup.org>
---
FATE.pm | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/FATE.pm b/FATE.pm
index 50b5c69..27bd960 100644
--- a/FATE.pm
+++ b/FATE.pm
@@ -28,9 +28,9 @@ BEGIN {
@EXPORT = qw/split_header split_config split_rec parse_date agestr
split_stats load_summary load_report load_lastpass
start end tag h1 span trow trowa trowh th td anchor
- head1 head2 head3 footer
- fail $fatedir $recent_age $ancient_age $hidden_age href
- $gitweb/;
+ head1 head2 head3 footer href
+ fail
+ $fatedir $recent_age $ancient_age $hidden_age $gitweb/;
}
our $fatedir = "/var/www/fateweb";
--
2.30.2
From a53e264717f6695834525edaa99c22cd8fb33a99 Mon Sep 17 00:00:00 2001
From: Nicolas George <geo...@nsup.org>
Date: Sun, 8 Aug 2021 14:20:13 +0200
Subject: [PATCH 02/11] *.cgi: hardcode Perl library path.
The environment will be considered unsafe under taint-cheeck rules.
The path is already hard-coded in FATE.pm.
Signed-off-by: Nicolas George <geo...@nsup.org>
---
history.cgi | 2 ++
index.cgi | 2 ++
log.cgi | 2 ++
report.cgi | 2 ++
4 files changed, 8 insertions(+)
diff --git a/history.cgi b/history.cgi
index cb6f71c..360d0fa 100755
--- a/history.cgi
+++ b/history.cgi
@@ -17,6 +17,8 @@
use strict;
use warnings;
+use lib "/var/www/fateweb";
+
use CGI qw/param/;
use FATE;
use Time::Zone;
diff --git a/index.cgi b/index.cgi
index a164d3b..2ad5d35 100755
--- a/index.cgi
+++ b/index.cgi
@@ -18,6 +18,8 @@
use strict;
use warnings;
+use lib "/var/www/fateweb";
+
use CGI qw/param/;
use HTML::Entities;
use FATE;
diff --git a/log.cgi b/log.cgi
index 8767e3a..ffd6000 100755
--- a/log.cgi
+++ b/log.cgi
@@ -17,6 +17,8 @@
use strict;
use warnings;
+use lib "/var/www/fateweb";
+
use CGI qw/param/;
use FATE;
diff --git a/report.cgi b/report.cgi
index a980617..ae510c6 100755
--- a/report.cgi
+++ b/report.cgi
@@ -17,6 +17,8 @@
use strict;
use warnings;
+use lib "/var/www/fateweb";
+
use POSIX qw/asctime/;
use CGI qw/param/;
use HTML::Entities;
--
2.30.2
From ffc968c8b4b95975988952c00e9aa1bfd0573fe6 Mon Sep 17 00:00:00 2001
From: Nicolas George <geo...@nsup.org>
Date: Sun, 8 Aug 2021 14:47:28 +0200
Subject: [PATCH 03/11] index: remove $uri.
A relative URI works just fine and is more convenient.
$ENV{REQUEST_URI} seems to be Apache-specific.
Signed-off-by: Nicolas George <geo...@nsup.org>
---
index.cgi | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/index.cgi b/index.cgi
index 2ad5d35..2931036 100755
--- a/index.cgi
+++ b/index.cgi
@@ -38,8 +38,6 @@ $sort =~ s/[^A-Za-z0-9 ]*//g;
param('sort', $sort);
$sort = $sort eq 'arch' ? 'subarch': $sort;
-(my $uri = $ENV{REQUEST_URI}) =~ s/\?.*//;
-
opendir D, $fatedir or fail 'Server error: $fatedir not found';
my @slots = grep /^[^.]/, readdir D;
closedir D;
@@ -77,7 +75,7 @@ for my $slot (@slots) {
}
@reps or fail @queries ? 'No items matching search criteria. ' .
- "<a href=\"$uri\">Clear all search criteria.</a>" :
+ "<a href=\"\">Clear all search criteria.</a>" :
'No data in $fatedir.';
$allpass = 100 * $allpass / @reps;
@@ -133,7 +131,7 @@ sub lsort {
}
$key = $newkey if $newkey ne '';
- anchor $text, href => "$uri?${params}sort=$key";
+ anchor $text, href => "?${params}sort=$key";
}
sub category {
@@ -164,7 +162,7 @@ sub category {
$head_printed = 1; # for the sake of completeness
start 'td';
- anchor $$rep{$category}, href => "$uri?$params";
+ anchor $$rep{$category}, href => "?$params";
end 'td';
}
@@ -207,7 +205,7 @@ if (@queries) {
my ($type, $text) = split(/:/, $this_query, 2);
print "$type: $text; ";
}
- anchor 'clear all.', href => "$uri";
+ anchor 'clear all.', href => "";
end 'p';
}
--
2.30.2
From fbea1454b447ea4a76fabe36d4ed7c0ca0b240de Mon Sep 17 00:00:00 2001
From: Nicolas George <geo...@nsup.org>
Date: Sun, 8 Aug 2021 15:20:37 +0200
Subject: [PATCH 04/11] FATE: add functions to validate parameters.
Signed-off-by: Nicolas George <geo...@nsup.org>
---
FATE.pm | 36 ++++++++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)
diff --git a/FATE.pm b/FATE.pm
index 27bd960..1b10564 100644
--- a/FATE.pm
+++ b/FATE.pm
@@ -19,6 +19,7 @@ use strict;
use warnings;
use POSIX qw/asctime mktime/;
+use CGI ();
BEGIN {
use Exporter;
@@ -30,6 +31,8 @@ BEGIN {
start end tag h1 span trow trowa trowh th td anchor
head1 head2 head3 footer href
fail
+ safeparam safeparam_opt
+ safeparam_sort safeparam_slot safeparam_time safeparam_log
$fatedir $recent_age $ancient_age $hidden_age $gitweb/;
}
@@ -442,4 +445,37 @@ sub fail {
exit 1;
}
+# safeparam qr/.../, $name
+# Validate parameter $name with a regular expression.
+sub safeparam_opt($$) {
+ my ($re, $n) = @_;
+ my $v = CGI::param($n);
+ return undef if !defined $v;
+ ($v) = $v =~ /^($re)\z/ or fail "Invalid $n value.";
+ return $v;
+}
+
+sub safeparam($$) {
+ my ($re, $n) = @_;
+ my $v = safeparam_opt $re, $n;
+ fail "No $n parameter." unless defined $v;
+ return $v;
+}
+
+sub safeparam_sort() {
+ return safeparam_opt qr/[a-z]{1,10}(?:\/\/[a-z]{1,10})*/, 'sort';
+}
+
+sub safeparam_slot() {
+ return safeparam qr/[A-Za-z0-9_\-.]{1,80}/, 'slot';
+}
+
+sub safeparam_time() {
+ return safeparam qr/[0-9]{14}/, 'time';
+}
+
+sub safeparam_log() {
+ return safeparam qr/[a-z]{1,10}(?:\/[0-9]{14})?/, 'log';
+}
+
1;
--
2.30.2
From 648ac7aef2c7eead6c6afd0f2703646b1ff64f96 Mon Sep 17 00:00:00 2001
From: Nicolas George <geo...@nsup.org>
Date: Sun, 8 Aug 2021 15:59:37 +0200
Subject: [PATCH 05/11] all: uniformize compression handling.
Add helper functions to FATE.pm.
Use the helper functions wherever relevant.
Add compression to history.cgi.
Signed-off-by: Nicolas George <geo...@nsup.org>
---
FATE.pm | 18 ++++++++++++++++++
history.cgi | 4 +++-
index.cgi | 7 +------
log.cgi | 6 ++----
report.cgi | 7 +------
5 files changed, 25 insertions(+), 17 deletions(-)
diff --git a/FATE.pm b/FATE.pm
index 1b10564..b570d64 100644
--- a/FATE.pm
+++ b/FATE.pm
@@ -31,6 +31,7 @@ BEGIN {
start end tag h1 span trow trowa trowh th td anchor
head1 head2 head3 footer href
fail
+ ready_for_gzip end_headers_and_compress
safeparam safeparam_opt
safeparam_sort safeparam_slot safeparam_time safeparam_log
$fatedir $recent_age $ancient_age $hidden_age $gitweb/;
@@ -445,6 +446,23 @@ sub fail {
exit 1;
}
+sub ready_for_gzip() {
+ my $ae = $ENV{HTTP_ACCEPT_ENCODING};
+ if (defined($ae) && $ae =~ /gzip/) {
+ print "Content-Encoding: gzip\r\n\r\n";
+ return 1;
+ } else {
+ print "\r\n";
+ return 0;
+ }
+}
+
+sub end_headers_and_compress() {
+ if (ready_for_gzip) {
+ open STDOUT, '|-', 'gzip';
+ }
+}
+
# safeparam qr/.../, $name
# Validate parameter $name with a regular expression.
sub safeparam_opt($$) {
diff --git a/history.cgi b/history.cgi
index 360d0fa..da577e0 100755
--- a/history.cgi
+++ b/history.cgi
@@ -35,7 +35,9 @@ close D;
@reps or fail "No data in $fatedir";
-print "Content-type: text/html\r\n\r\n";
+print "Content-type: text/html\r\n";
+
+end_headers_and_compress;
head1;
print "<title>FATE: $slot</title>\n";
diff --git a/index.cgi b/index.cgi
index 2931036..48cd076 100755
--- a/index.cgi
+++ b/index.cgi
@@ -169,12 +169,7 @@ sub category {
print "Content-type: text/html\r\n";
print "Access-Control-Allow-Origin: https://ffmpeg.org\r\n";;
-if ($ENV{HTTP_ACCEPT_ENCODING} =~ /gzip/) {
- print "Content-Encoding: gzip\r\n\r\n";
- open STDOUT, '|-', 'gzip';
-} else {
- print "\r\n";
-}
+end_headers_and_compress;
head1;
print "<title>FATE</title>\n";
diff --git a/log.cgi b/log.cgi
index ffd6000..cf538fe 100755
--- a/log.cgi
+++ b/log.cgi
@@ -43,16 +43,14 @@ if (! -r $log) {
if ($req_diff) {
my $dlog = "$fatedir/$req_slot/$req_diff/$req_log.log.gz";
- print "\r\n";
+ end_headers_and_compress;
exec 'zdiff', '-u', $dlog, $log;
}
my $cat = 'zcat';
-if ($ENV{HTTP_ACCEPT_ENCODING} =~ /gzip/) {
- print "Content-Encoding: gzip\r\n";
+if (ready_for_gzip) {
$cat = 'cat';
}
-print "\r\n";
exec $cat, $log;
diff --git a/report.cgi b/report.cgi
index ae510c6..f5cc6ca 100755
--- a/report.cgi
+++ b/report.cgi
@@ -58,12 +58,7 @@ my $lastpass = load_lastpass $req_slot;
print "Content-type: text/html\r\n";
-if ($ENV{HTTP_ACCEPT_ENCODING} =~ /gzip/) {
- print "Content-Encoding: gzip\r\n\r\n";
- open STDOUT, '|-', 'gzip';
-} else {
- print "\r\n";
-}
+end_headers_and_compress;
head1;
print "<title>FATE: $$hdr{slot} $$hdr{rev}</title>\n";
--
2.30.2
From 37e31b5297cf05635ac387b1632d8b9c47d429ac Mon Sep 17 00:00:00 2001
From: Nicolas George <geo...@nsup.org>
Date: Sun, 8 Aug 2021 17:07:20 +0200
Subject: [PATCH 06/11] history: validate parameters.
Signed-off-by: Nicolas George <geo...@nsup.org>
---
history.cgi | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/history.cgi b/history.cgi
index da577e0..4ab9543 100755
--- a/history.cgi
+++ b/history.cgi
@@ -19,12 +19,11 @@ use warnings;
use lib "/var/www/fateweb";
-use CGI qw/param/;
use FATE;
use Time::Zone;
use HTML::Entities;
-my $slot = param 'slot';
+my $slot = safeparam_slot;
my $slotdir = "$fatedir/$slot";
my $slot_escaped = encode_entities $slot;
--
2.30.2
From f9f77da3dcbeacb7b3cc2c96bdcbfe35b6bb4dce Mon Sep 17 00:00:00 2001
From: Nicolas George <geo...@nsup.org>
Date: Sun, 8 Aug 2021 15:22:32 +0200
Subject: [PATCH 07/11] report: validate parameters.
Signed-off-by: Nicolas George <geo...@nsup.org>
---
report.cgi | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/report.cgi b/report.cgi
index f5cc6ca..3c93709 100755
--- a/report.cgi
+++ b/report.cgi
@@ -20,13 +20,12 @@ use warnings;
use lib "/var/www/fateweb";
use POSIX qw/asctime/;
-use CGI qw/param/;
use HTML::Entities;
use MIME::Base64;
use FATE;
-my $req_slot = param 'slot';
-my $req_time = param 'time';
+my $req_slot = safeparam_slot;
+my $req_time = safeparam_time;
my $slotdir = "$fatedir/$req_slot";
my $repdir = "$slotdir/$req_time";
--
2.30.2
From cb02a9c52b554693c070a930dbdb60b0cd58275b Mon Sep 17 00:00:00 2001
From: Nicolas George <geo...@nsup.org>
Date: Sun, 8 Aug 2021 17:07:36 +0200
Subject: [PATCH 08/11] log: validate parameters.
Signed-off-by: Nicolas George <geo...@nsup.org>
---
log.cgi | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/log.cgi b/log.cgi
index cf538fe..6442b84 100755
--- a/log.cgi
+++ b/log.cgi
@@ -19,16 +19,13 @@ use warnings;
use lib "/var/www/fateweb";
-use CGI qw/param/;
use FATE;
-my $req_slot = param 'slot';
-my $req_time = param 'time';
-$req_slot =~ s/[^-._A-Za-z0-9 ]*//g;
-$req_time =~ s/[^0-9]*//g;
-my ($req_log, $req_diff) = param('log') =~ m!([^/]+)(?:/([^/]+))?!;
-$req_log =~ s/[^a-z]*//g;
-$req_diff =~ s/[^0-9]*//g;
+my $req_slot = safeparam_slot;
+my $req_time = safeparam_time;
+my $req_log = safeparam_log;
+my $req_diff;
+$req_diff = $1 if $req_log =~ s/\/(.*)//;
my $repdir = "$fatedir/$req_slot/$req_time";
my $log = "$repdir/$req_log.log.gz";
--
2.30.2
From aa96a582550429b70bf9753a898025da872b7310 Mon Sep 17 00:00:00 2001
From: Nicolas George <geo...@nsup.org>
Date: Sun, 8 Aug 2021 14:33:20 +0200
Subject: [PATCH 09/11] index: validate parameters.
query still needs to be validated, but it is only used
for strign operations, and the matter of double escaping
needs to be clarified first.
Signed-off-by: Nicolas George <geo...@nsup.org>
---
index.cgi | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/index.cgi b/index.cgi
index 48cd076..880de3c 100755
--- a/index.cgi
+++ b/index.cgi
@@ -33,10 +33,8 @@ use URI::Escape;
# split(/:/, $this_query, 2);
my @queries = split(/\/\//, uri_unescape param 'query') if (param 'query');
-my $sort = param('sort');
-$sort =~ s/[^A-Za-z0-9 ]*//g;
-param('sort', $sort);
-$sort = $sort eq 'arch' ? 'subarch': $sort;
+my $sort = safeparam_sort;
+$sort = "subarch" if defined($sort) && $sort eq "arch";
opendir D, $fatedir or fail 'Server error: $fatedir not found';
my @slots = grep /^[^.]/, readdir D;
--
2.30.2
From c372fd7887c6afa4d63a62068887f308d7634b81 Mon Sep 17 00:00:00 2001
From: Nicolas George <geo...@nsup.org>
Date: Sun, 8 Aug 2021 16:04:40 +0200
Subject: [PATCH 10/11] *.cgi: enable taint checks.
Sanitize $ENV{PATH} in the function of FATE.pm that
ends the HTTP headers.
Signed-off-by: Nicolas George <geo...@nsup.org>
---
FATE.pm | 2 ++
history.cgi | 2 +-
index.cgi | 2 +-
log.cgi | 2 +-
4 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/FATE.pm b/FATE.pm
index b570d64..6d55df7 100644
--- a/FATE.pm
+++ b/FATE.pm
@@ -447,6 +447,8 @@ sub fail {
}
sub ready_for_gzip() {
+ # Under CGI, $PATH is safe
+ ($ENV{PATH}) = $ENV{PATH} =~ /(.*)/s;
my $ae = $ENV{HTTP_ACCEPT_ENCODING};
if (defined($ae) && $ae =~ /gzip/) {
print "Content-Encoding: gzip\r\n\r\n";
diff --git a/history.cgi b/history.cgi
index 4ab9543..95ba23a 100755
--- a/history.cgi
+++ b/history.cgi
@@ -1,4 +1,4 @@
-#! /usr/bin/perl
+#! /usr/bin/perl -T
#
# Copyright (c) 2011 Mans Rullgard <m...@mansr.com>
#
diff --git a/index.cgi b/index.cgi
index 880de3c..a805607 100755
--- a/index.cgi
+++ b/index.cgi
@@ -1,4 +1,4 @@
-#! /usr/bin/perl
+#! /usr/bin/perl -T
#
# Copyright (c) 2011 Mans Rullgard <m...@mansr.com>
# Copyright (c) 2014 Tiancheng "Timothy" Gu <timothyg...@gmail.com>
diff --git a/log.cgi b/log.cgi
index 6442b84..5fe108b 100755
--- a/log.cgi
+++ b/log.cgi
@@ -1,4 +1,4 @@
-#! /usr/bin/perl
+#! /usr/bin/perl -T
#
# Copyright (c) 2011 Mans Rullgard <m...@mansr.com>
#
--
2.30.2
From 9c934067e9e8daad77db00913d60ac1841e23c44 Mon Sep 17 00:00:00 2001
From: Nicolas George <geo...@nsup.org>
Date: Sun, 8 Aug 2021 19:15:17 +0200
Subject: [PATCH 11/11] index: force scalar context for param('query').
Silence a warning; no security problem, the extra parameter
would just pollute the third argument of split.
Signed-off-by: Nicolas George <geo...@nsup.org>
---
index.cgi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/index.cgi b/index.cgi
index a805607..17adabe 100755
--- a/index.cgi
+++ b/index.cgi
@@ -31,7 +31,7 @@ use URI::Escape;
# @queries contains an array of 'type:value' strings.
# Every member of @queries can be further parsed with another simple
# split(/:/, $this_query, 2);
-my @queries = split(/\/\//, uri_unescape param 'query') if (param 'query');
+my @queries = split(/\/\//, uri_unescape scalar param 'query') if (param 'query');
my $sort = safeparam_sort;
$sort = "subarch" if defined($sort) && $sort eq "arch";
--
2.30.2
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
