Re: [PATCH] Do not overflow string buffer (PR objc/85476).

[Jakub Jelinek]

On Fri, Apr 20, 2018 at 11:44:35AM +0200, Martin Liška wrote:
> Hi.
> 
> Quite obvious package that causes an ASAN error described in the PR.
> 
> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
> 
> Ready to be installed?
> Martin
> 
> gcc/objc/ChangeLog:
> 
> 2018-04-20  Martin Liska  
> 
>   PR objc/85476
>   * objc-act.c (finish_class): Do not overflow string buffer.

Ok, thanks.

Jakub

Re: [PATCH] Do not overflow string buffer (PR objc/85476).

[Martin Sebor]

On 04/20/2018 03:44 AM, Martin Liška wrote:

Hi.

Quite obvious package that causes an ASAN error described in the PR.

Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.


As an aside, I went and looked at the rest of code to see if
the overflow could be detected at compile time and if it could
be why it's not.  Here's what the code boils down to:

  void f (char*);

  void g (const char *s)
  {
 unsigned n = strlen (s);
 char *d = alloca (n);
 strcpy (d, s);
 f (d);
  }

Even though the off-by-one error is obvious it's not detected
either with _FORTIFY_SOURCE or without.  Both fail because
compute_builtin_object_size() only detects constant sizes.

But the strlen pass tracks both the size of allocations and
the lengths of even non-constant strings (computed by strlen)
so detecting the overflow there should be straightforward.
In the test case above the pass sees the following:

  _1 = __builtin_strlen (s_4(D));
  _9 = _1 & 4294967295;
  d_6 = __builtin_alloca (_9);
  __builtin_strcpy (d_6, s_4(D));

I've raised bug 85484 to try to implement this in GCC 9.

(Another way to handle this would be to enhance builtin-object
size to track non-constant sizes but that would require bigger
changes).

Martin



Ready to be installed?
Martin

gcc/objc/ChangeLog:

2018-04-20  Martin Liska  

PR objc/85476
* objc-act.c (finish_class): Do not overflow string buffer.
---
 gcc/objc/objc-act.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Re: [PATCH] Do not overflow string buffer (PR objc/85476).

[Richard Biener]

On Fri, Apr 20, 2018 at 11:44 AM, Martin Liška  wrote:
> Hi.
>
> Quite obvious package that causes an ASAN error described in the PR.
>
> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
>
> Ready to be installed?

Ok.

Richard.

> Martin
>
> gcc/objc/ChangeLog:
>
> 2018-04-20  Martin Liska  
>
> PR objc/85476
> * objc-act.c (finish_class): Do not overflow string buffer.
> ---
>  gcc/objc/objc-act.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
>

[PATCH] Do not overflow string buffer (PR objc/85476).

[Martin Liška]

Hi.

Quite obvious package that causes an ASAN error described in the PR.

Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.

Ready to be installed?
Martin

gcc/objc/ChangeLog:

2018-04-20  Martin Liska  

PR objc/85476
* objc-act.c (finish_class): Do not overflow string buffer.
---
 gcc/objc/objc-act.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


diff --git a/gcc/objc/objc-act.c b/gcc/objc/objc-act.c
index b87f7cc075e..d08693051ea 100644
--- a/gcc/objc/objc-act.c
+++ b/gcc/objc/objc-act.c
@@ -8003,7 +8003,7 @@ finish_class (tree klass)
 		char *setter_name = (char *) alloca (length);
 		tree ret_type, selector, arg_type, arg_name;
 
-		strcpy (setter_name, full_setter_name);
+		memcpy (setter_name, full_setter_name, length - 1);
 		setter_name[length - 1] = '\0';
 		ret_type = build_tree_list (NULL_TREE, void_type_node);
 		arg_type = build_tree_list (NULL_TREE, TREE_TYPE (x));