[pve-devel] [PATCH access-control 2/2] fix #1499: check ACL path validity

[Thomas Lamprecht]

On 10/10/2017 03:44 PM, Philip Abernethy wrote:
> Checks ACL paths for logical validity before application. Checks of
> the various IDs are done by the existing format checkers to avoid code
> duplication.
> Also introduces a distinction between malformed (syntactically
> incorrect) and invalid (syntactically correct, but contextually wrong)
> paths.

Just for the record, this fixes 1500 not 1499:
https://bugzilla.proxmox.com/show_bug.cgi?id=1500

1500 is intended for the backend check (i.e., this fix) where 1499
was opened for the frontend UI.

Thanks for tackling this, though!

> ---
>  PVE/API2/ACL.pm      |  4 +++-
>  PVE/AccessControl.pm | 13 +++++++++++++
>  2 files changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/PVE/API2/ACL.pm b/PVE/API2/ACL.pm
> index d37771b..20d3d2a 100644
> --- a/PVE/API2/ACL.pm
> +++ b/PVE/API2/ACL.pm
> @@ -132,7 +132,9 @@ __PACKAGE__->register_method ({
>  	}
>  
>  	my $path = PVE::AccessControl::normalize_path($param->{path});
> -	raise_param_exc({ path => "invalid ACL path '$param->{path}'" }) if !$path;
> +	raise_param_exc({ path => "malformed ACL path '$param->{path}'" }) if !$path;
> +	raise_param_exc({ path => "invalid ACL path '$param->{path}'" })
> +	    if !PVE::AccessControl::validate_path($path);
>  
>  	PVE::AccessControl::lock_user_config(
>  	    sub {
> diff --git a/PVE/AccessControl.pm b/PVE/AccessControl.pm
> index f0fb7dc..183bf21 100644
> --- a/PVE/AccessControl.pm
> +++ b/PVE/AccessControl.pm
> @@ -591,6 +591,19 @@ sub verify_privname {
>      return $priv;
>  }
>  
> +sub validate_path {
> +    my $path = shift;
> +    return 0 if $path !~ m'^/(vms|nodes|storage|pool|access/(?:groups|realms))(?:/([[:alnum:]\.\-\_]+))?$';
> +
> +    if ($1 eq 'vms') {PVE::JSONSchema::pve_verify_vmid($2) if $2;}
> +    elsif ($1 eq 'nodes') {PVE::JSONSchema::pve_verify_node_name($2) if $2;}
> +    elsif ($1 eq 'storage') {PVE::JSONSchema::parse_storage_id($2) if $2;}
> +    elsif ($1 eq 'pool') {verify_poolname($2) if $2;}
> +    elsif ($1 eq 'access/realms') {PVE::Auth::Plugin::pve_verify_realm($2) if $2;}
> +
> +    return 1;
> +}
> +
>  sub userconfig_force_defaults {
>      my ($cfg) = @_;
>  
>