From 37d3c16b25644f953647b512d9d231c866fa94e0 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Fabian=20Gr=C3=BCnbichler?= Date: Mon, 20 Jun 2022 13:05:12 +0200 Subject: [PATCH] perm check: forbid undefined/empty ACL path MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit to detect similar issues to that fixed in the previous commit early on and without the potential for dangerous side-effects. root@pam is intentionally still allowed before the check in case such situations can be triggered by misconfiguration - root@pam can then still clean up the affected configs via the GUI/API, and not just via manual editing. Signed-off-by: Fabian Grünbichler --- src/PVE/API2/AccessControl.pm | 1 + src/PVE/AccessControl.pm | 12 ++++++++++++ src/PVE/RPCEnvironment.pm | 13 +++++++++++-- 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/src/PVE/API2/AccessControl.pm b/src/PVE/API2/AccessControl.pm index 5d78c6f..104e7e3 100644 --- a/src/PVE/API2/AccessControl.pm +++ b/src/PVE/API2/AccessControl.pm @@ -115,6 +115,7 @@ my sub verify_auth : prototype($$$$$$$) { my ($rpcenv, $username, $pw_or_ticket, $otp, $path, $privs, $new_format) = @_; my $normpath = PVE::AccessControl::normalize_path($path); + die "invalid path - $path\n" if defined($path) && !defined($normpath); my $ticketuser; if (($ticketuser = PVE::AccessControl::verify_ticket($pw_or_ticket, 1)) && diff --git a/src/PVE/AccessControl.pm b/src/PVE/AccessControl.pm index a2e0458..3725a7d 100644 --- a/src/PVE/AccessControl.pm +++ b/src/PVE/AccessControl.pm @@ -498,6 +498,8 @@ my $assemble_short_lived_ticket = sub { $path = normalize_path($path); + die "invalid ticket path\n" if !defined($path); + my $secret_data = "$username:$path"; return PVE::Ticket::assemble_rsa_ticket( @@ -509,6 +511,8 @@ my $verify_short_lived_ticket = sub { $path = normalize_path($path); + die "invalid ticket path\n" if !defined($path); + my $secret_data = "$username:$path"; my ($rsa_pub, $rsa_mtime) = get_pubkey(); @@ -1166,6 +1170,8 @@ sub lookup_username { sub normalize_path { my $path = shift; + return undef if !$path; + $path =~ s|/+|/|g; $path =~ s|/$||; @@ -1641,6 +1647,12 @@ sub roles { return 'Administrator' if $user eq 'root@pam'; # root can do anything + if (!defined($path)) { + # this shouldn't happen! + warn "internal error: ACL check called for undefined ACL path!\n"; + return {}; + } + if (pve_verify_tokenid($user, 1)) { my $tokenid = $user; my ($username, $token) = split_tokenid($tokenid); diff --git a/src/PVE/RPCEnvironment.pm b/src/PVE/RPCEnvironment.pm index e03974a..0ee2346 100644 --- a/src/PVE/RPCEnvironment.pm +++ b/src/PVE/RPCEnvironment.pm @@ -140,6 +140,12 @@ sub permissions { return { map { $_ => 1 } keys %{$cfg->{roles}->{'Administrator'}} }; } + if (!defined($path)) { + # this shouldn't happen! + warn "internal error: ACL check called for undefined ACL path!\n"; + return {}; + } + if (PVE::AccessControl::pve_verify_tokenid($user, 1)) { my ($username, $token) = PVE::AccessControl::split_tokenid($user); my $cfg = $self->{user_cfg}; @@ -439,8 +445,10 @@ sub exec_api2_perm_check { raise_perm_exc(); } my $path = PVE::Tools::template_replace($tmplpath, $param); - $path = PVE::AccessControl::normalize_path($path); - return $self->check_full($username, $path, $privs, $any, $noerr); + my $normpath = PVE::AccessControl::normalize_path($path); + warn "Failed to normalize '$path'\n" if !defined($normpath) && defined($path); + + return $self->check_full($username, $normpath, $privs, $any, $noerr); } elsif ($test eq 'userid-group') { my $userid = $param->{userid}; my ($t, $privs, %options) = @$check; @@ -490,6 +498,7 @@ sub exec_api2_perm_check { my ($t, $tmplpath) = @$check; my $path = PVE::Tools::template_replace($tmplpath, $param); $path = PVE::AccessControl::normalize_path($path); + return 0 if !defined($path); # should already die in API2::ACL return $self->check_perm_modify($username, $path, $noerr); } else { die "unknown permission test"; -- 2.39.2