[pve-devel] applied: [PATCH http-server] decode_urlencoded: cope with undefined values

[Thomas Lamprecht]

Avoids syslog/journal warning like:
>  Use of uninitialized value $v in substitution (s///) at
>  /usr/share/perl5/PVE/APIServer/AnyEvent.pm line 648.

If one passes a "value-less" GET argument to a request, e.g.,
GET /?debug

Besides the fact that this allows us to even use such arguments it
also is a general improvement against a slight "syslog DOS attack",
because anybody can pass such parameters to the '/' page, and all
proxmox daemons providing a API/UI using libpve-http-server-perl
allow to do such requests unauthenticated (which itself is OK, as
else one could not show the login window at all). As each of such
request produces two log lines in the syslog/journal it's far from
ideal.

A simple reproducer of the possible outcome can be seen with the
following shell script using curl:

> PVEURL='127.0.0.1'
> ARGS='?a'; # send multiple args at once to amplify the per-connection cost
> for c in {a..z}; do for i in {0..9}; do ARGS="$ARGS&$c$i"; done; done
> while true; do curl --insecure --silent --output /dev/null "https://$PVEURL:8006$ARGS"; done

Not really bad, but not nice either, as logging is not too cheap this
has some resource usage cost and noise in the syslog is never nice.

Signed-off-by: Thomas Lamprecht <t.lamprecht at proxmox.com>
---

applied directly to master and stable-5

 PVE/APIServer/AnyEvent.pm | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/PVE/APIServer/AnyEvent.pm b/PVE/APIServer/AnyEvent.pm
index c6b74c0..2e8ca47 100644
--- a/PVE/APIServer/AnyEvent.pm
+++ b/PVE/APIServer/AnyEvent.pm
@@ -645,16 +645,19 @@ sub decode_urlencoded {
 	my ($k, $v) = split(/=/, $kv);
 	$k =~s/\+/ /g;
 	$k =~ s/%([0-9a-fA-F][0-9a-fA-F])/chr(hex($1))/eg;
-	$v =~s/\+/ /g;
-	$v =~ s/%([0-9a-fA-F][0-9a-fA-F])/chr(hex($1))/eg;
 
-	$v = Encode::decode('utf8', $v);
+	if (defined($v)) {
+	    $v =~s/\+/ /g;
+	    $v =~ s/%([0-9a-fA-F][0-9a-fA-F])/chr(hex($1))/eg;
 
-	if (defined(my $old = $res->{$k})) {
-	    $res->{$k} = "$old\0$v";
-	} else {
-	    $res->{$k} = $v;
+	    $v = Encode::decode('utf8', $v);
+
+	    if (defined(my $old = $res->{$k})) {
+		$v = "$old\0$v";
+	    }
 	}
+
+	$res->{$k} = $v;
     }
     return $res;
 }
-- 
2.20.1