Chad Fraleigh (12021-08-22): > It mostly looks good (from a perl perspective).
Thanks for the comments.
>
> Just 3 questionable items..
>
> -<>-<>-
>
> -if ($ENV{HTTP_ACCEPT_ENCODING} =~ /gzip/) {
> - print "Content-Encoding: gzip\r\n";
> +if (ready_for_gzip) {
> $cat = 'cat';
> }
>
> The old code outputs "\r\n", where ready_for_gzip() outputs "\r\n\r\n". Will
> this be an issue, or should it have done that in the first place?
There is a “-print "\r\n";” just a little below, that balances out.
>
> -<>-<>-
>
> +sub ready_for_gzip() {
> + my $ae = $ENV{HTTP_ACCEPT_ENCODING};
> + if (defined($ae) && $ae =~ /gzip/) {
>
> It is checking for 'gzip' as a substring, rather than a whole word. If it
> was passed a [hypothetical] encoding type contains the substring gzip (e.g.
> "bigzip"), it could trigger in incompatible output encoding. However, it's
> not any worse than it was previously.
>
> Perhaps changing it to match /\bgzip\b/ ?
I wanted minimalistic changes, but this is better.
>
> -<>-<>-
>
> sub ready_for_gzip() {
> + # Under CGI, $PATH is safe
> + ($ENV{PATH}) = $ENV{PATH} =~ /(.*)/s;
>
> It is untainting the PATH as "hidden" side effect of calling
> ready_for_gzip(). While technically it works, it feels a little kludgy
> compared to untainting it at the beginning of each taint-enabled script.
You are right. Changed.
I will try to deploy the changes this shortly.
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.32.0
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.32.0
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.32.0
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.32.0
From 2a8ee058d0b4bb0857ec94d327a4ccd86a3e3792 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..3813ab2 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 =~ /\bgzip\b/) {
+ 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.32.0
From 6e901f09cb99d3600caab24ff9a1d2d5b0979c3f 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.32.0
From 0562b8be06e05abd44468cc7ce95f38965e59179 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.32.0
From 8bd488abb8b273a369c7b11b19cf69c2df2382bc 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.32.0
From e23cf69fc05b7fb58829f9f6eae26393fcac043b 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.32.0
From 505f620a5d22ffef86ad5ffa1328e87ba6dc191b 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 | 5 +++++
history.cgi | 4 +++-
index.cgi | 4 +++-
log.cgi | 4 +++-
report.cgi | 2 ++
5 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/FATE.pm b/FATE.pm
index 3813ab2..df45f4a 100644
--- a/FATE.pm
+++ b/FATE.pm
@@ -32,6 +32,7 @@ BEGIN {
head1 head2 head3 footer href
fail
ready_for_gzip end_headers_and_compress
+ cgi_path_is_trustworthy
safeparam safeparam_opt
safeparam_sort safeparam_slot safeparam_time safeparam_log
$fatedir $recent_age $ancient_age $hidden_age $gitweb/;
@@ -446,6 +447,10 @@ sub fail {
exit 1;
}
+sub cgi_path_is_trustworthy() {
+ ($ENV{PATH}) = $ENV{PATH} =~ /(.*)/s;
+}
+
sub ready_for_gzip() {
my $ae = $ENV{HTTP_ACCEPT_ENCODING};
if (defined($ae) && $ae =~ /\bgzip\b/) {
diff --git a/history.cgi b/history.cgi
index 4ab9543..e38672f 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>
#
@@ -23,6 +23,8 @@ use FATE;
use Time::Zone;
use HTML::Entities;
+cgi_path_is_trustworthy;
+
my $slot = safeparam_slot;
my $slotdir = "$fatedir/$slot";
diff --git a/index.cgi b/index.cgi
index 880de3c..5be5097 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>
@@ -26,6 +26,8 @@ use FATE;
use Time::Zone;
use URI::Escape;
+cgi_path_is_trustworthy;
+
# Format for /?query= : /?query=type:value//type:value// (URI encoded).
# Trailing // does not matter (i.e. may be added).
# @queries contains an array of 'type:value' strings.
diff --git a/log.cgi b/log.cgi
index 6442b84..8e41c7b 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>
#
@@ -21,6 +21,8 @@ use lib "/var/www/fateweb";
use FATE;
+cgi_path_is_trustworthy;
+
my $req_slot = safeparam_slot;
my $req_time = safeparam_time;
my $req_log = safeparam_log;
diff --git a/report.cgi b/report.cgi
index 3c93709..2b388e4 100755
--- a/report.cgi
+++ b/report.cgi
@@ -24,6 +24,8 @@ use HTML::Entities;
use MIME::Base64;
use FATE;
+cgi_path_is_trustworthy;
+
my $req_slot = safeparam_slot;
my $req_time = safeparam_time;
--
2.32.0
From ade2ddcbe85947d245543d0bb3b94ce67295ab79 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 5be5097..8fe92db 100755
--- a/index.cgi
+++ b/index.cgi
@@ -33,7 +33,7 @@ cgi_path_is_trustworthy;
# @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.32.0
signature.asc
Description: PGP signature
_______________________________________________ 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".
