]> git.proxmox.com Git - pve-http-server.git/commitdiff
decode_urlencoded: cope with undefined values
authorThomas Lamprecht <t.lamprecht@proxmox.com>
Tue, 9 Jul 2019 12:04:42 +0000 (14:04 +0200)
committerThomas Lamprecht <t.lamprecht@proxmox.com>
Tue, 9 Jul 2019 12:34:52 +0000 (14:34 +0200)
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@proxmox.com>
(cherry picked from commit 4a09623bbdb4306ca00685a0f80ce6f05fa42f3b)
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
PVE/APIServer/AnyEvent.pm

index cb9c52d156dc76afdfdfc8b29b5bfe1552bac4c6..a9d61c8224c20ea8ba9c9cbd189b429676f0120b 100644 (file)
@@ -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;
 }