From 4a09623bbdb4306ca00685a0f80ce6f05fa42f3b Mon Sep 17 00:00:00 2001 From: Thomas Lamprecht Date: Tue, 9 Jul 2019 14:04:42 +0200 Subject: [PATCH] decode_urlencoded: cope with undefined values 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 --- 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.39.2