asan: heap buffer overflow in pa_chk_field_selector

asan: heap buffer overflow in pa_chk_field_selector

[Alan Modra]

The buffer overflow showed up running the gas "all macro" test.

	* config/tc-hppa.c (pa_chk_field_selector): Don't read past end
	of line.

diff --git a/gas/config/tc-hppa.c b/gas/config/tc-hppa.c
index 742d262a5b5..5a4db51b89a 100644
--- a/gas/config/tc-hppa.c
+++ b/gas/config/tc-hppa.c
@@ -2432,24 +2432,37 @@ pa_chk_field_selector (char **str)
   int middle, low, high;
   int cmp;
   char name[4];
+  char *s = *str;
 
   /* Read past any whitespace.  */
-  /* FIXME: should we read past newlines and formfeeds??? */
-  while (**str == ' ' || **str == '\t' || **str == '\n' || **str == '\f')
-    *str = *str + 1;
-
-  if ((*str)[1] == '\'' || (*str)[1] == '%')
-    name[0] = TOLOWER ((*str)[0]),
-    name[1] = 0;
-  else if ((*str)[2] == '\'' || (*str)[2] == '%')
-    name[0] = TOLOWER ((*str)[0]),
-    name[1] = TOLOWER ((*str)[1]),
-    name[2] = 0;
-  else if ((*str)[3] == '\'' || (*str)[3] == '%')
-    name[0] = TOLOWER ((*str)[0]),
-    name[1] = TOLOWER ((*str)[1]),
-    name[2] = TOLOWER ((*str)[2]),
-    name[3] = 0;
+  while (*s == ' ' || *s == '\t')
+    s++;
+  *str = s;
+
+  if (is_end_of_line [(unsigned char) s[0]])
+    return e_fsel;
+  else if (s[1] == '\'' || s[1] == '%')
+    {
+      name[0] = TOLOWER (s[0]);
+      name[1] = 0;
+    }
+  else if (is_end_of_line [(unsigned char) s[1]])
+    return e_fsel;
+  else if (s[2] == '\'' || s[2] == '%')
+    {
+      name[0] = TOLOWER (s[0]);
+      name[1] = TOLOWER (s[1]);
+      name[2] = 0;
+    }
+  else if (is_end_of_line [(unsigned char) s[2]])
+    return e_fsel;
+  else if (s[3] == '\'' || s[3] == '%')
+    {
+      name[0] = TOLOWER (s[0]);
+      name[1] = TOLOWER (s[1]);
+      name[2] = TOLOWER (s[2]);
+      name[3] = 0;
+    }
   else
     return e_fsel;
 

-- 
Alan Modra
Australia Development Lab, IBM

Re: asan: heap buffer overflow in pa_chk_field_selector

[Jan Beulich]

On 29.03.2022 02:58, Alan Modra via Binutils wrote:
> The buffer overflow showed up running the gas "all macro" test.
> 
> 	* config/tc-hppa.c (pa_chk_field_selector): Don't read past end
> 	of line.

Thanks for taking care of this. It was in particular my intent to avoid
checking for end-of-line which first made me hesitant to fix this myself.
But I see you dropped the oddity at the same time, including the FIXME.
Before seeing your fix, I did actually make one using !ISALPHA() instead
of is_end_of_line(), but now I can right away drop that again.

Jan

> --- a/gas/config/tc-hppa.c
> +++ b/gas/config/tc-hppa.c
> @@ -2432,24 +2432,37 @@ pa_chk_field_selector (char **str)
>    int middle, low, high;
>    int cmp;
>    char name[4];
> +  char *s = *str;
>  
>    /* Read past any whitespace.  */
> -  /* FIXME: should we read past newlines and formfeeds??? */
> -  while (**str == ' ' || **str == '\t' || **str == '\n' || **str == '\f')
> -    *str = *str + 1;
> -
> -  if ((*str)[1] == '\'' || (*str)[1] == '%')
> -    name[0] = TOLOWER ((*str)[0]),
> -    name[1] = 0;
> -  else if ((*str)[2] == '\'' || (*str)[2] == '%')
> -    name[0] = TOLOWER ((*str)[0]),
> -    name[1] = TOLOWER ((*str)[1]),
> -    name[2] = 0;
> -  else if ((*str)[3] == '\'' || (*str)[3] == '%')
> -    name[0] = TOLOWER ((*str)[0]),
> -    name[1] = TOLOWER ((*str)[1]),
> -    name[2] = TOLOWER ((*str)[2]),
> -    name[3] = 0;
> +  while (*s == ' ' || *s == '\t')
> +    s++;
> +  *str = s;
> +
> +  if (is_end_of_line [(unsigned char) s[0]])
> +    return e_fsel;
> +  else if (s[1] == '\'' || s[1] == '%')
> +    {
> +      name[0] = TOLOWER (s[0]);
> +      name[1] = 0;
> +    }
> +  else if (is_end_of_line [(unsigned char) s[1]])
> +    return e_fsel;
> +  else if (s[2] == '\'' || s[2] == '%')
> +    {
> +      name[0] = TOLOWER (s[0]);
> +      name[1] = TOLOWER (s[1]);
> +      name[2] = 0;
> +    }
> +  else if (is_end_of_line [(unsigned char) s[2]])
> +    return e_fsel;
> +  else if (s[3] == '\'' || s[3] == '%')
> +    {
> +      name[0] = TOLOWER (s[0]);
> +      name[1] = TOLOWER (s[1]);
> +      name[2] = TOLOWER (s[2]);
> +      name[3] = 0;
> +    }
>    else
>      return e_fsel;
>  
>