From a5320155c19a9ce8a0cf1e3802705ae05884ef2f Mon Sep 17 00:00:00 2001 From: Stoiko Ivanov Date: Fri, 6 Aug 2021 17:44:27 +0200 Subject: [PATCH] acme client: fix #3536 untaint data returned from acme server The data returned from the acme server (e.g. boulder) should be considered tainted. To see which places might need untainted I checked where $self->{ua} was used (in $self->do) and a quick scan identified __get_result as the consumer of the tainted data. In all but one uses the data is decoded from json (which would die if the result is not valid json). The remaining use-case yields a certificate in PEM format (and is handled at the caller of __get_result). The issue is currently only visible if a proxy is set, because AFAICT somewhere in SSLeay (or IO::Socket::SSL, which uses SSLeay) a taint flag is not set on the return value. A reproducer for the issue: ``` use strict; use warnings; use HTTP::Request; use LWP::UserAgent; $ENV{PATH} = "/usr/bin:/bin"; my $ua = LWP::UserAgent->new(env_proxy => 0); my $request = HTTP::Request->new('GET', 'https://google.com/'); my $resp = $ua->request($request); my $text = substr($resp->decoded_content, 0, 5);; system("echo \"$text\""); # does work $request = HTTP::Request->new('GET', 'http://neverssl.com/'); $resp = $ua->request($request); $text = substr($resp->decoded_content, 0, 5);; system("echo \"$text\""); # does not work ``` Signed-off-by: Stoiko Ivanov --- src/PVE/ACME.pm | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/PVE/ACME.pm b/src/PVE/ACME.pm index c5b8d66..265482d 100644 --- a/src/PVE/ACME.pm +++ b/src/PVE/ACME.pm @@ -69,7 +69,9 @@ sub tojs($;%) { # shortcut for to_json with utf8=>1 } sub fromjs($) { - return from_json($_[0]); + my ($data) = @_; + ($data) = ($data =~ /^(.*)$/s); # untaint from_json croaks on error anyways. + return from_json($data); } sub fatal($$;$$) { @@ -449,7 +451,13 @@ sub get_certificate { if !$order->{certificate}; my $r = $self->do(POST => $order->{certificate}, ''); - my $return = eval { __get_result($r, 200, 1); }; + my $return = eval { + my $res = __get_result($r, 200, 1); + if ($res =~ /^(-----BEGIN CERTIFICATE-----)(.+)(-----END CERTIFICATE-----)$/s) { # untaint + return $1 . $2 . $3; + } + die "Server reply does not look like a PEM encoded certificate\n"; + }; $self->fatal("POST of '$order->{certificate}' failed - $@", $r) if $@; return $return; } -- 2.39.2