acme client: fix #3536 untaint data returned from acme server
authorStoiko Ivanov <s.ivanov@proxmox.com>
Fri, 6 Aug 2021 15:44:27 +0000 (17:44 +0200)
committerFabian Gr├╝nbichler <f.gruenbichler@proxmox.com>
Wed, 11 Aug 2021 09:52:53 +0000 (11:52 +0200)
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@proxmox.com>
src/PVE/ACME.pm

index c5b8d669f8b1496e1c451cb4d0ff264c519ec97e..265482d8bc6434b2db94d1025ccd204cd487abfc 100644 (file)
@@ -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;
 }