djantzen has asked for the wisdom of the Perl Monks concerning the following question:

Since we seem to have a security theme building here this morning, I've decided to post a subroutine I've been working on. It's part of a module, actually the root module in a hierarchy of three, that enables arbitrary actions to be taken at later dates. Subclasses expand this behavior to include execution at specific, possibly recurring dates, and a mini-'language' for constructing the more common sorts of jobs. It's also capable of contacting users for purposes of notification and approval. Before continuing, I want to stress two points:

  1. This is an in-house system and under no circumstances will this, say, end up sitting on a public Web server with a nice friendly CGI interface.
  2. Most of the time these modules will be reached via other in-house applications, thereby further limiting access.

Now, in our system we have various usertypes. Every user has at least one assigned to him/her, and it's common to have more. There are three primary groups I'm concerned with: programmers, editors, and consultants. Programmers ought to be able to do whatever they want; editors are a highly trusted bunch, but still ought to have some restrictions; consultants are at the outskirts of the organization and have a high turnover rate, and so, while allowed to have some access to this system, need to have some heavy restrictions placed upon them. Furthermore, these usertypes may be modified by the attributes 'former' or 'admin' or 'trainee' (more precisely: ('former) || ('admin' || 'trainee')). Thus, we have things like 'former consultant', 'editor admin', 'programmer trainee', etc. I for example am a former editor and a programmer. What I want to do is limit the sorts of code that a user can run by tying particular opsets to trust levels assigned by usertype. This is accomplished using the Safe module. In my case, the fact that I'm a former editor should win me no trust, however the fact that I'm a programmer should enable me to do whatever I want. So, the assignment has to be smart enough to give trust where and only where it's due, and take it away in the same manner.

That said, here's the code:

sub _permittedOps() { my ($this) = @_; # Hashes for mapping in order to avoid if-else chaining my (%attr_value, %high_type, %trust_level); # A map associating opcode sets with trust levels. %trust_level = (full => full_opset(), strong => opset(qw/:default :filesys_read :sys_db :filesys_open :filesys_write :ownproc +ess :subprocess/), normal => opset(qw/:default/), weak => opset(qw/:base_core :base_mem :base_loop :base_io :base_orig/), none => opset(qw/:base_core :base_loop/) ); # A map associating usertype attributes with positive/negative val +ues # affecting the level of trust assigned to the user. %attr_value = (admin => 1, consultant => -1, editor => 0, former = +> -2, other => -2, programmer => 2, trainee => -1); # Run through all of the assigned usertypes, adjusting the trust l +evel in %high_type # granted the user for each particular usertype. $1, if defined wi +ll be # 'former', $2 will be either 'consultant', 'editor', or 'programm +er'. # $3, if defined, will be either 'admin' or 'trainee'. foreach my $type (@usertypes) { # @usertypes is defined in the rea +l code, don't worry :) if ($type =~ /^(former)?\s?(consultant|editor|programmer)\s? (admin|trainee)?$/ix) { unless ($1) { # Look up the appropriate usertype and attribute values. $high_type{$2} += $attr_value{$2}; $high_type{$2} += $attr_value{$3} if $3; } else { $high_type{$2} += $attr_value{$1} } } else { $high_type{other} += $attr_value{other}; } } # Take the maximum trust available. my $max = pop(@{[sort values %high_type]}); # Return the appropriate opset. ($max > 1) and return $trust_level{full}; ($max == 1) and return $trust_level{strong}; ($max == 0) and return $trust_level{normal}; ($max == -1) and return $trust_level{weak}; ($max < -1) and return $trust_level{none}; }

What I'd like is some feedback on a few separate issues:

  1. Is this a sane breakdown of opsets?
  2. Are there holes in the algorithm that might enable escalation of privileges?
  3. Is this readable/maintainable?

Many thanks in advance, fever. Oh and it's my birthday so be nice :^P

Replies are listed 'Best First'.
Re: Assigning Opsets by User Type
by Abigail-II (Bishop) on Nov 20, 2002 at 16:13 UTC
    One obvious possible loophole is that there's no check done whether @usertypes contains duplicates. Someone who is managed to trick the system believing he's twice an "admin editor" gets the same priviledges as a "programmer".

    Also, you're doing a sort values %high_type, which is a lexical sort - but you want to sort numerically. It happens to go right because ord ("-") < ord ("0"), and you don't have numbers larger than 9 involved. But it ain't "maintainable".

    Some maintainance problems: the regex used to breakdown the usertypes uses the same hardcoded keys as in %attr_value. The order in which the keys are put in %attr_value is messy. There are several "configuration" thingies (content of %attr_value, the trust levels, and what they are allowed to do), scattered all over the body of the subroutine. I'd group them, and preferably, take them outside of the sub.

    Also, from your introduction, it appears that a "former programmer admin" is an illegal user type. But your sub will give it a strong trust_level.

    Abigail

      Thanks Abigail. @usertypes should be okay due to checks in the module from which it comes, although I'll have to audit that code to make sure. The sorting bit now does a sort { $a <=> $b }.

      The order in which the keys are put in %attr_value is messy. Well, I don't think it's messy per se, I mean it's alphabetical, but perhaps it would be clearer to group them into negative forces, neutral, and positive.

      it appears that a "former programmer admin" is an illegal user type. But your sub will give it a strong trust_level.

      This is an interesting worry. On the one hand, I've got a variety of ways to solve it, from doing another if-else to see if there's a violation of the 'former' || ('admin' || 'trainee') rule (icky), or I could make 'former' be worth a greater negative value (hackish), or do a forward lookahead in the regex I suppose (not bad). But on the other hand, these values come from UserType objects whose values are constrained by a database table, so to construct an invalid usertype would require admin privileges on the DB. And at that point the system is totally compromised anyway.

      Thanks again for the feedback!

        Why the urge to solve the parsing with a single regex? I would solve it differently. I wouldn't use %attr_value to group "tags" like 'former', 'admin' and 'trainee' together with the role. I would so something like:
        my %roles = qw /consultant -1 editor 0 programmer 2/; my %pre_attrs = qw /former -2/; my %post_attrs = qw /trainee -1 admin 1/; my $def_score = -2; my @trust = ([full => 2], [strong => 1], [normal => 0], [weak => -1], my $def_trust = 'none'; my $max_score = -10000; # Some big, negative number. foreach my $type (@usertypes) { my $score = 0; my $attrs = 0; while (my ($key, $value) = each %pre_attrs) { if ($type =~ s/\Q$key\E\s*//) { $score += $value; $saw_pre = 1; last; } } while (my ($key, $value) = each %roles) { if ($type =~ s/\Q$key\E\s*//) { $score += $value; $saw_role = 1; last; } } while (my ($key, $value) = each %post_attrs) { if ($type =~ s/\Q$key\E\s*//) { $score += $value; $saw_post = 1; last; } } if (length $type || $saw_pre && $saw_post || !$saw_role) { # Illegal or unknown usertype. $score = $def_score; } $max_score = $score if $score > $max_score; } foreach my $level (@trust) { return $level -> [0] if $max_score >= $level -> [1]; } return $def_trust;

        Abigail