public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Stoiko Ivanov <s.ivanov@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [PATCH proxmox-acme 1/3] acme client: fix #3536 untaint data returned from acme server
Date: Fri,  6 Aug 2021 17:44:27 +0200	[thread overview]
Message-ID: <20210806154429.1675997-2-s.ivanov@proxmox.com> (raw)
In-Reply-To: <20210806154429.1675997-1-s.ivanov@proxmox.com>

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 | 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





  reply	other threads:[~2021-08-06 15:44 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-06 15:44 [pve-devel] [PATCH proxmox-acme 0/3] update acme.sh and fix #3536 and #3546 Stoiko Ivanov
2021-08-06 15:44 ` Stoiko Ivanov [this message]
2021-08-06 15:44 ` [pve-devel] [PATCH proxmox-acme 2/3] update to acme.sh dns plugins to 3.0.0 Stoiko Ivanov
2021-08-06 15:44 ` [pve-devel] [PATCH proxmox-acme 3/3] plugin-caller: pull in changes from upstream 3.0.0 Stoiko Ivanov
2021-08-11 10:29 ` [pve-devel] applied: [PATCH proxmox-acme 0/3] update acme.sh and fix #3536 and #3546 Fabian Grünbichler

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210806154429.1675997-2-s.ivanov@proxmox.com \
    --to=s.ivanov@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal