[pve-devel] [PATCH proxmox-acme 1/3] acme client: fix #3536 untaint data returned from acme server

[Stoiko Ivanov]

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 <s.ivanov at proxmox.com>
---
 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.30.2