operations invert order of variable contents (= glob issues)

[Blub]

`add_var_object` moves the var_object pointer the new element was added to to point to the new end of the list, and set operations walk the lists backwards, this causes the order to be inverted on each operation, causing globs to get messed up.

Consider:

Code: Select all

define a {
  /dir
  /dir/foo*      r
  /dir/f*        rw
}
define b {
  /dummy
}

subject /works {
  $a | $b | $b
}
subject /fails {
  $a | $b
}


The /fails subject will error with

Code: Select all

Error on line 2517 of /etc/grsec/policy: Globbed object /dir/foo* in subject /fails is completely matched by previous globbed object /dir/f*.  As globbed objects with the same anchor are matched on a first-rule-matches-first policy, the ordering present in your policy likely does not reflect your intentions.


gradm version 3.0.201405281853

[Blub]

I noticed that interpret_variable in gradm_sym.c rewinds the var_object before using it so I took the liberty of preparing a patch doing the same for the 3 set operations, which seems to fix the problem.

Code: Select all

From 0e4dd8618cecc74dd91954b856a66bc8cc6abf5b Mon Sep 17 00:00:00 2001
From: Wolfgang Bumiller <wry.git@bumiller.com>
Date: Wed, 16 Jul 2014 16:40:01 +0200
Subject: [PATCH] rewind var_object pointers to head and perform set operations
 forward instead of backwards to not flip their contents on each operation

---
 gradm_sym.c | 31 +++++++++++++++++++++++--------
 1 file changed, 23 insertions(+), 8 deletions(-)

diff --git a/gradm_sym.c b/gradm_sym.c
index 7b552cb..d8fa7e6 100644
--- a/gradm_sym.c
+++ b/gradm_sym.c
@@ -60,10 +60,15 @@ struct var_object * intersect_objects(struct var_object *var1, struct var_object
 {
    struct var_object *tmpvar1, *tmpvar2, *retvar = NULL;
 
-   for (tmpvar1 = var1; tmpvar1; tmpvar1 = tmpvar1->prev) {
+   while (var1->prev)
+      var1 = var1->prev;
+   while (var2->prev)
+      var2 = var2->prev;
+
+   for (tmpvar1 = var1; tmpvar1; tmpvar1 = tmpvar1->next) {
       switch (tmpvar1->type) {
       case VAR_FILE_OBJECT:
-         for (tmpvar2 = var2; tmpvar2; tmpvar2 = tmpvar2->prev) {
+         for (tmpvar2 = var2; tmpvar2; tmpvar2 = tmpvar2->next) {
             switch (tmpvar2->type) {
             case VAR_FILE_OBJECT:
                if (!strcmp(tmpvar1->file_obj.filename, tmpvar2->file_obj.filename)) {
@@ -89,11 +94,16 @@ struct var_object * union_objects(struct var_object *var1, struct var_object *va
    struct var_object *tmpvar1, *tmpvar2, *retvar = NULL;
    int found_dupe = 0;
 
-   for (tmpvar1 = var1; tmpvar1; tmpvar1 = tmpvar1->prev) {
+   while (var1->prev)
+      var1 = var1->prev;
+   while (var2->prev)
+      var2 = var2->prev;
+
+   for (tmpvar1 = var1; tmpvar1; tmpvar1 = tmpvar1->next) {
       switch (tmpvar1->type) {
       case VAR_FILE_OBJECT:
          found_dupe = 0;
-         for (tmpvar2 = var2; tmpvar2; tmpvar2 = tmpvar2->prev) {
+         for (tmpvar2 = var2; tmpvar2; tmpvar2 = tmpvar2->next) {
             switch (tmpvar2->type) {
             case VAR_FILE_OBJECT:
                if (!strcmp(tmpvar1->file_obj.filename, tmpvar2->file_obj.filename)) {
@@ -114,11 +124,11 @@ struct var_object * union_objects(struct var_object *var1, struct var_object *va
       }
    }
 
-   for (tmpvar2 = var2; tmpvar2; tmpvar2 = tmpvar2->prev) {
+   for (tmpvar2 = var2; tmpvar2; tmpvar2 = tmpvar2->next) {
       switch (tmpvar2->type) {
       case VAR_FILE_OBJECT:
          found_dupe = 0;
-         for (tmpvar1 = var1; tmpvar1; tmpvar1 = tmpvar1->prev) {
+         for (tmpvar1 = var1; tmpvar1; tmpvar1 = tmpvar1->next) {
             switch (tmpvar1->type) {
             case VAR_FILE_OBJECT:
                if (!strcmp(tmpvar1->file_obj.filename, tmpvar2->file_obj.filename)) {
@@ -148,13 +158,18 @@ struct var_object * differentiate_objects(struct var_object *var1, struct var_ob
    int found_dupe = 0;
    char *path;
 
-   for (tmpvar1 = var1; tmpvar1; tmpvar1 = tmpvar1->prev) {
+   while (var1->prev)
+      var1 = var1->prev;
+   while (var2->prev)
+      var2 = var2->prev;
+
+   for (tmpvar1 = var1; tmpvar1; tmpvar1 = tmpvar1->next) {
       switch (tmpvar1->type) {
       case VAR_FILE_OBJECT:
          path = gr_strdup(tmpvar1->file_obj.filename);
          found_dupe = 0;
          do {
-            for (tmpvar2 = var2; tmpvar2; tmpvar2 = tmpvar2->prev) {
+            for (tmpvar2 = var2; tmpvar2; tmpvar2 = tmpvar2->next) {
                switch (tmpvar2->type) {
                case VAR_FILE_OBJECT:
                   if (!strcmp(path, tmpvar2->file_obj.filename)) {
--
2.0.1



[spender]

Wolfgang,

Thanks very much! You are officially my favorite new grsec user -- I can count on one hand I think how many people have sent patches Sorry I didn't get around to it last night.

-Brad

[Blub]

People must be be afraid of fiddling with security related code
Glad I can help