Blob Blame History Raw
From dfada9dfe4545ee19806d8812a85b194b90f08a0 Mon Sep 17 00:00:00 2001
From: Adrian Reber <areber@redhat.com>
Date: Tue, 18 Jan 2022 16:49:40 +0000
Subject: [PATCH 114/120] compel: fix GCC 12 failure (out of bounds)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This is a confusing change as it seems the original code was just wrong.
GCC 12 complains with:

In function ‘__conv_val’,
    inlined from ‘std_strtoul’ at compel/plugins/std/string.c:202:7:
compel/plugins/std/string.c:154:24: error: array subscript 97 is above array bounds of ‘const char[37]’ [-Werror=array-bounds]
  154 |                 return &conv_tab[__tolower(c)] - conv_tab;
      |                        ^~~~~~~~~~~~~~~~~~~~~~~
compel/plugins/std/string.c: In function ‘std_strtoul’:
compel/plugins/std/string.c:10:19: note: while referencing ‘conv_tab’
   10 | static const char conv_tab[] = "0123456789abcdefghijklmnopqrstuvwxyz";
      |                   ^~~~~~~~
cc1: all warnings being treated as errors

Which sounds correct. The array conv_tab has just 37 elements.

If I understand the code correctly we are trying to convert anything
that is character between a-z and A-Z to a number for cases where
the base is larger than 10. For a base 11 conversion b|B should return 11.
For a base 35 conversion z|Z should return 35. This is all for a strtoul()
implementation.

The original code was:

  static const char conv_tab[] = "0123456789abcdefghijklmnopqrstuvwxyz";

  return &conv_tab[__tolower(c)] - conv_tab;

and that seems wrong. If conv_tab would have been some kind of hash it could
have worked, but '__tolower()' will always return something larger than
97 ('a') which will always overflow the array.

But maybe I just don't get that part of the code.

I replaced it with

  return __tolower(c) - 'a' + 10;

which does the right thing: 'A' = 10, 'B' = 11 ... 'Z' = 35

Signed-off-by: Adrian Reber <areber@redhat.com>
---
 compel/plugins/std/string.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/compel/plugins/std/string.c b/compel/plugins/std/string.c
index bde1bc68b..d67e0d1a9 100644
--- a/compel/plugins/std/string.c
+++ b/compel/plugins/std/string.c
@@ -151,7 +151,12 @@ static unsigned int __conv_val(unsigned char c)
 	if (__isdigit(c))
 		return c - '0';
 	else if (__isalpha(c))
-		return &conv_tab[__tolower(c)] - conv_tab;
+		/**
+		 * If we want the value of something which __isalpha() == true
+		 * it has to be base > 10. 'A' = 10, 'B' = 11 ... 'Z' = 35
+		 */
+		return __tolower(c) - 'a' + 10;
+
 	return -1u;
 }
 
-- 
2.34.1