RFC PATCH: glob backslash handling and recursive ** glob

Discuss and suggest new grsecurity features

RFC PATCH: glob backslash handling and recursive ** glob

Postby Blub » Sat Jun 04, 2016 12:38 pm

Couldn't find a devel mailing list so I'm pasting patches here again. (Been a few years ;-).)

First patch changes the '\' case since it was special-cased but behaved like a literal backslash which seemed somewhat misleading. Also gradm_analyze.c doesn't use FNM_NOESCAPE when checking for patterns completely matched by previous ones, so this seemed like a discrepancy.

The second is mostly a "wish" and could use some auditing. It makes "**" match recursively.

These are for the kernel patch and change glob_match(). To generate the patches (and do *some* testing) I moved the function out into a git repo with just that function in a test file, so I can't guarantee the `patch` tool can apply them to your working code base.

I'd also like to add support for {foo,bar} to gradm (which can be done purely in gradm) if such a patch would be acceptable. This would allow things like:
Code: Select all
replace RESOLVCONF /{etc,{var/,}run/NetworkManager}/resolv.conf


I've also noticed that gradm allows using quotes to eg. include spaces, but quotes don't escape globs which seems counter intuitive, so I was wondering if it would be desirable to make this more shell-like and make quotes escape globs and allow mixing quoted strings with non-quoted strings. Like in a shell you can do: "/some path"/*/"with spaces".
I realize that making quotes escape globs would be a "breaking" change, though, so a flag or enable/disable rule for this behavior would probably be desirable. Especially since currently the lexer treats "a"b"c" as if the inner two quotes are part of the name (iow. like the shell would interpret "a\"b\"c") while "a"r with no space in between treats 'r' as the mode for the object "a" while other letters error.

Code: Select all
From 468b64e997e3c615715aed699e71aa28491eda04 Mon Sep 17 00:00:00 2001
From: Wolfgang Bumiller <[email protected]>
Date: Sat, 4 Jun 2016 12:13:07 +0200
Subject: [RFC PATCH 1/2] glob_match: make '\' an escape character

Having a separate 'case' statement for slashes without
giving them the usual escape meaning is a somewhat
misleading code piece.
The previous slash case worked like passing FNM_NOESCAPE to
fnmatch() which is neither what users expect, nor what
gradm() checks against in its analyzer.

A trailing backslash is still treated as a literal backslash.

Signed-off-by: Wolfgang Bumiller <[email protected]>
---
 glob_match.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/glob_match.c b/glob_match.c
index 0d954e2..45614e0 100644
--- a/glob_match.c
+++ b/glob_match.c
@@ -20,7 +20,10 @@ glob_match(const char *p, const char *n)
             return 1;
          break;
       case '\\':
-         if (*n != c)
+         if (*p) {
+            if (*n != *p++)
+               return 1;
+         } else if (*n != c)
             return 1;
          break;
       case '*':
--
2.8.3



Code: Select all
From 9f5d9dcc2aa4b694c97800a4e53153a88daa4f61 Mon Sep 17 00:00:00 2001
From: Wolfgang Bumiller <[email protected]>
Date: Sat, 4 Jun 2016 12:50:56 +0200
Subject: [RFC PATCH 2/2] support recursive matches via **

Signed-off-by: Wolfgang Bumiller <[email protected]>
---
 glob_match.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/glob_match.c b/glob_match.c
index 45614e0..6b4ae02 100644
--- a/glob_match.c
+++ b/glob_match.c
@@ -10,6 +10,7 @@ static int
 glob_match(const char *p, const char *n)
 {
    char c;
+   bool recursive;
 
    while ((c = *p++) != '\0') {
    switch (c) {
@@ -27,8 +28,10 @@ glob_match(const char *p, const char *n)
             return 1;
          break;
       case '*':
+         recursive = false;
          for (c = *p++; c == '?' || c == '*'; c = *p++) {
-            if (*n == '/')
+            recursive = (c == '*');
+            if (!recursive && *n == '/')
                return 1;
             else if (c == '?') {
                if (*n == '\0')
@@ -42,7 +45,7 @@ glob_match(const char *p, const char *n)
          } else {
             const char *endp;
 
-            if ((endp = strchr(n, '/')) == NULL)
+            if (recursive || (endp = strchr(n, '/')) == NULL)
                endp = n + strlen(n);
 
             if (c == '[') {
@@ -50,10 +53,12 @@ glob_match(const char *p, const char *n)
                   if (!glob_match(p, n))
                      return 0;
             } else if (c == '/') {
-               while (*n != '\0' && *n != '/')
-                  ++n;
-               if (*n == '/' && !glob_match(p, n + 1))
-                  return 0;
+               do {
+                  while (*n != '\0' && *n != '/')
+                     ++n;
+                  if (*n == '/' && !glob_match(p, n + 1))
+                     return 0;
+               } while (*n++ == '/' && recursive);
             } else {
                for (--p; n < endp; ++n)
                   if (*n == c && !glob_match(p, n))
--
2.8.3



On an unrelated note... while writing this post I really wish I could use [cmd] or [font=Monospace] or something other than enclosing quote-examples with more confusing quotation ;-)
Blub
 
Posts: 9
Joined: Tue Jul 15, 2014 4:38 am

Return to grsecurity development