From 5f6f9a1beab327be2728d44c1996408176f6800e Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Wed, 11 May 2016 09:18:14 +0100 Subject: Wrapper: allocate new environment dynamically Otherwise, if third-party plugins extend newenviron by more than 3 entries, we could overflow the array. It seems unlikely that any third-party plugin manipulates newenviron in practice, so this is mostly theoretical. Just in case, I have deliberately avoided using "i" as the variable name, so that any third-party plugin that was manipulating newenviron directly will now result in the wrapper failing to compile. I have not assumed that realloc(NULL, ...) works as an equivalent of malloc(...), in case there are still operating systems where that doesn't work. --- IkiWiki/Receive.pm | 4 +-- IkiWiki/Wrapper.pm | 37 ++++++++++++++------- debian/changelog | 7 ++++ t/cvs.t | 2 +- t/wrapper-environ.t | 92 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 127 insertions(+), 15 deletions(-) create mode 100755 t/wrapper-environ.t diff --git a/IkiWiki/Receive.pm b/IkiWiki/Receive.pm index 5908e09f9..332ba7c2c 100644 --- a/IkiWiki/Receive.pm +++ b/IkiWiki/Receive.pm @@ -46,8 +46,8 @@ EOF while (read(0, &buf, 256) != 0) {} exit(0); } - asprintf(&s, "CALLER_UID=%i", u); - newenviron[i++]=s; + asprintf(&s, "%i", u); + addenv("CALLER_UID", s); } EOF return $ret; diff --git a/IkiWiki/Wrapper.pm b/IkiWiki/Wrapper.pm index 69500029c..a8de39eea 100644 --- a/IkiWiki/Wrapper.pm +++ b/IkiWiki/Wrapper.pm @@ -52,7 +52,6 @@ sub gen_wrapper () { HTTP_COOKIE REMOTE_USER HTTPS REDIRECT_STATUS HTTP_HOST SERVER_PORT HTTPS HTTP_ACCEPT REDIRECT_URL} if $config{cgi}; - my $envsize=$#envsave; my $envsave=""; foreach my $var (@envsave) { $envsave.=<<"EOF"; @@ -65,7 +64,6 @@ EOF my $val=$config{ENV}{$key}; utf8::encode($val) if utf8::is_utf8($val); $val =~ s/([^A-Za-z0-9])/sprintf '""\\x%02x""', ord($1)/ge; - $envsize += 1; $envsave.=<<"EOF"; addenv("$key", "$val"); EOF @@ -184,18 +182,33 @@ EOF #include extern char **environ; -char *newenviron[$envsize+7]; -int i=0; +int newenvironlen=0; +/* Array of length newenvironlen+1 (+1 for NULL) */ +char **newenviron=NULL; void addenv(char *var, char *val) { - char *s=malloc(strlen(var)+1+strlen(val)+1); + char *s; + + if (newenviron) { + newenviron=realloc(newenviron, (newenvironlen+2) * sizeof(char *)); + } + else { + newenviron=calloc(newenvironlen+2, sizeof(char *)); + } + + if (!newenviron) { + perror("realloc"); + exit(1); + } + + s=malloc(strlen(var)+1+strlen(val)+1); if (!s) { perror("malloc"); exit(1); } else { sprintf(s, "%s=%s", var, val); - newenviron[i++]=s; + newenviron[newenvironlen++]=s; } } @@ -215,9 +228,9 @@ int main (int argc, char **argv) { $check_commit_hook @wrapper_hooks $envsave - newenviron[i++]="HOME=$ENV{HOME}"; - newenviron[i++]="PATH=$ENV{PATH}"; - newenviron[i++]="WRAPPED_OPTIONS=$configstring"; + addenv("HOME", "$ENV{HOME}"); + addenv("PATH", "$ENV{PATH}"); + addenv("WRAPPED_OPTIONS", "$configstring"); #ifdef __TINYC__ /* old tcc versions do not support modifying environ directly */ @@ -225,10 +238,10 @@ $envsave perror("clearenv"); exit(1); } - for (; i>0; i--) - putenv(newenviron[i-1]); + for (; newenvironlen>0; newenvironlen--) + putenv(newenviron[newenvironlen-1]); #else - newenviron[i]=NULL; + newenviron[newenvironlen]=NULL; environ=newenviron; #endif diff --git a/debian/changelog b/debian/changelog index 81a679282..e721f167f 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,10 @@ +ikiwiki (3.20160510) UNRELEASED; urgency=medium + + * wrappers: allocate new environment dynamically, so we won't overrun + the array if third-party plugins add multiple environment variables. + + -- Simon McVittie Wed, 11 May 2016 09:15:51 +0100 + ikiwiki (3.20160509) unstable; urgency=high [ Amitai Schlair ] diff --git a/t/cvs.t b/t/cvs.t index 371c21ec9..6acafd701 100755 --- a/t/cvs.t +++ b/t/cvs.t @@ -708,5 +708,5 @@ sub stripext { } sub _wrapper_paths { - return qq{newenviron[i++]="PERL5LIB=$ENV{PERL5LIB}";}; + return qq{addenv("PERL5LIB", "$ENV{PERL5LIB}");}; } diff --git a/t/wrapper-environ.t b/t/wrapper-environ.t new file mode 100755 index 000000000..ddf5a6e44 --- /dev/null +++ b/t/wrapper-environ.t @@ -0,0 +1,92 @@ +#!/usr/bin/perl +use warnings; +use strict; + +use Test::More; +plan(skip_all => "IPC::Run not available") + unless eval q{ + use IPC::Run qw(run); + 1; + }; + +use IkiWiki; + +use Cwd qw(getcwd); +use Errno qw(ENOENT); + +my $installed = $ENV{INSTALLED_TESTS}; + +my @command; +if ($installed) { + @command = qw(env PERL5LIB=t/tmp ikiwiki); +} +else { + ok(! system("make -s ikiwiki.out")); + @command = qw(env PERL5LIB=t/tmp:blib/lib:blib/arch perl -I. ./ikiwiki.out + --underlaydir=underlays/basewiki + --set underlaydirbase=underlays + --templatedir=templates); +} + +writefile("test.setup", "t/tmp", < "getsetup", id => "excessiveenvironment", call => \&getsetup); + hook(type => "genwrapper", id => "excessiveenvironment", call => \&genwrapper); +} + +sub getsetup { + return plugin => { + safe => 0, + rebuild => undef, + section => "rcs", + }; +} + +sub genwrapper { + my @ret; + foreach my $j (1..4096) { + push @ret, qq{addenv("VAR$j", "val$j");\n}; + } + return join '', @ret; +} + +1; +EOF + ); + +my $stdout; +ok(! system(@command, qw(--setup t/tmp/test.setup --rebuild --wrappers)), "run ikiwiki"); +ok(run(["./t/tmp/ikiwiki.cgi"], '<&-', '>', \$stdout, init => sub { + $ENV{HTTP_HOST} = "localhost"; + $ENV{QUERY_STRING} = "do=prefs"; + $ENV{REQUEST_METHOD} = "GET"; + $ENV{SCRIPT_NAME} = "/cgi-bin/ikiwiki.cgi"; + $ENV{SERVER_PORT} = "80" +}), "run CGI"); + +done_testing(); -- cgit v1.2.3