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

[Philip Abernethy]

On Tue, Oct 10, 2017 at 03:53:45PM +0200, Thomas Lamprecht wrote:
> 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.

Oops, true. I can submit a v2 if desired. However this probably fixes
both bugs, as the resulting exception is displayed on the WebUI and CLI
as well.
> 
> 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) = @_;
> >  
> > 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel