[PATCH] Fix known_glibc_specials comparison

[PATCH] Fix known_glibc_specials comparison

[Tulio Magno Quites Machado Filho]

From: Tulio Magno Quites Machado Filho <tuliom@redhat.com>

Sometimes variable path stores a longer path instead of just the name of
a file, e.g. /usr/lib/libfoo.so instead of libfoo.so.
In those cases, we need to remove all the directories before calling
strcmp().
---
 annocheck/hardened.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/annocheck/hardened.c b/annocheck/hardened.c
index 604b6a1..2463101 100644
--- a/annocheck/hardened.c
+++ b/annocheck/hardened.c
@@ -2273,9 +2273,15 @@ is_special_glibc_binary (annocheck_data * data)
       "zic"
     };
 
+  char * file = strrchr(path, '/');
+  if (file == NULL)
+    file = (char *) path;
+  else
+    /* We also want to ignore the last '/' in the string.  */
+    file++;
   for (i = ARRAY_SIZE (known_glibc_specials); i--;)
     {
-      int res = strcmp (path, known_glibc_specials[i]);
+      int res = strcmp (file, known_glibc_specials[i]);
 
       if (res == 0)
 	return true;
-- 
2.45.1

Re: [PATCH] Fix known_glibc_specials comparison

[Nick Clifton]

[-- Attachment #1: Type: text/plain, Size: 832 bytes --]

Hi Tulio,

> Sometimes variable path stores a longer path instead of just the name of
> a file, e.g. /usr/lib/libfoo.so instead of libfoo.so.

Err, I think, based upon your code you mean that path should be "usr/lib/libfoo.so"
instead of just "libfoo.so".  (ie without the leading forward slash - there is
already code at the start of is_special_glibc_binary that handles absolute paths).

> In those cases, we need to remove all the directories before calling
> strcmp().

I am reluctant to accept this change because it would treat, eg "build/getent"
as a glibc binary when it almost certainly should not.  I would prefer it if
only a known set of path prefixes were allowed.

Instead, would you mind trying out the attached alternative patch and let me
know if it solves the problem that you were investigating ?

Cheers
   Nick

[-- Attachment #2: glibc-abs-path.patch --]
[-- Type: text/x-patch, Size: 1533 bytes --]

diff --git a/annocheck/hardened.c b/annocheck/hardened.c
index 604b6a1..deeb084 100644
--- a/annocheck/hardened.c
+++ b/annocheck/hardened.c
@@ -1895,30 +1895,32 @@ is_special_glibc_binary (annocheck_data * data)
 	  return true;
     }
 
-  /* If we are testing an uninstalled rpm then the paths will start with "."
-     so skip this.  */
+  /* If we are testing an uninstalled rpm then the paths will probably
+     start with "." so skip this.  */
   if (path[0] == '.')
     ++path;
-
   if (path[0] == '/')
+    ++path;
+  /* Look for absolute paths to known glibc install locations.
+     If found, strip the prefix.
+     This allows us to cope with symbolic links and 32-bit/64-bit multilibs.  */
+  if (strchr (path, '/'))
     {
-      /* If the path is absolute, then strip the prefix.
-	 This allows us to cope with symbolic links and 32-bit/64-bit multilibs.  */
       static const char * known_prefixes [] =
 	{
 	  /* NB/ The terminating forward slash is important.  */
-	  "/lib/",
-	  "/lib64/",
-	  "/sbin/",
-	  "/usr/bin/",
-	  "/usr/lib/",
-	  "/usr/lib/gconv/",
-	  "/usr/lib64/",
-	  "/usr/lib64/gconv/",
-	  "/usr/lib64/glibc-hwcaps/power10/",
-	  "/usr/libexec/",
-	  "/usr/libexec/getconf/",
-	  "/usr/sbin/"
+	  "lib/",
+	  "lib64/",
+	  "sbin/",
+	  "usr/bin/",
+	  "usr/lib/",
+	  "usr/lib/gconv/",
+	  "usr/lib64/",
+	  "usr/lib64/gconv/",
+	  "usr/lib64/glibc-hwcaps/power10/",
+	  "usr/libexec/",
+	  "usr/libexec/getconf/",
+	  "usr/sbin/"
 	};
 
       for (i = ARRAY_SIZE (known_prefixes); i--;)

Re: [PATCH] Fix known_glibc_specials comparison

[Tulio Magno Quites Machado Filho]

Nick Clifton <nickc@redhat.com> writes:

> Instead, would you mind trying out the attached alternative patch and let me
> know if it solves the problem that you were investigating ?

Yes, it does!

Thank you!

-- 
Tulio Magno