Bug 19390 - Integer overflow in strncat
Summary: Integer overflow in strncat
Status: RESOLVED FIXED
Alias: None
Product: glibc
Classification: Unclassified
Component: string (show other bugs)
Version: 2.22
: P2 normal
Target Milestone: 2.25
Assignee: Adhemerval Zanella
URL:
Keywords:
: 17279 (view as bug list)
Depends on:
Blocks:
 
Reported: 2015-12-21 20:51 UTC by Alexander Cherepanov
Modified: 2017-01-03 16:26 UTC (History)
5 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:
cherepan: security+


Attachments
Test strncat for all offsets (704 bytes, text/x-csrc)
2015-12-26 19:09 UTC, Alexander Cherepanov
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Alexander Cherepanov 2015-12-21 20:51:53 UTC
Apparently, on x86_64 (and other archs?) the asm version of strncat has an integer overflow similar to bug 19387.

sysdeps/x86_64/multiarch/strcat-sse2-unaligned.S is used on my machine. I didn't look into the details but it looks like strncat(s1, s2, n) misbehave when n is near SIZE_MAX, strlen(s2) >= 34 and s2 has specific offset.

For example, the program:

----------------------------------------------------------------------

#include <stdint.h>
#include <stdalign.h>
#include <string.h>
#include <stdio.h>

int main()
{
  alignas(64) char s[144];
  memset(s, 1, sizeof s);

  /* the first string... */
  char *s1 = s;             /* ...is at the start of the buffer */
  s1[0] = 0;                /* ...and is empty */

  /* the second string...  */
  char *s2 = s + 95;            /* ...starts as pos 95,  */
  memset(s2, 2, s + sizeof s - s2); /* ...filled with 2s for contrast */
  s2[33] = 0;                   /* ...and has the length 33 */

  printf("before:\n");
  for (int i = 0; i < 50; i++)
    printf("%x", (unsigned char)s[i]);
  printf("...\n");

  strncat(s1, s2, SIZE_MAX);

  printf("after:\n");
  for (int i = 0; i < 50; i++)
    printf("%x", (unsigned char)s[i]);
  printf("...\n");
  printf("%-33s^\n", "the string should end here");
}

----------------------------------------------------------------------

outputs this:

----------------------------------------------------------------------

before:
01111111111111111111111111111111111111111111111111...
after:
22222222222222222222222222222222202222211111111111...
the string should end here       ^

----------------------------------------------------------------------

Here, strncat put '\0' exactly where it should be but also copied 5 extra chars from s2 into s1 after '\0'. In other cases it copies less chars than required.

Checked on x86_64:
- git master (glibc-2.22-616-g5537f46) -- failed;
- Debian jessie (glibc 2.19-18+deb8u1) -- failed;
- Debian wheezy (eglibc 2.13-38+deb7u8) -- ok.

Checked on x86_64 with gcc -m32:
- Debian jessie (glibc 2.19-18+deb8u1) -- failed;
- Debian wheezy (eglibc 2.13-38+deb7u8) -- ok.

I didn't look into the details of 32-bit version.

The bug can have evident security implications but using strncat with n=SIZE_MAX seems rare hence filing it publicly.
Comment 1 howey014 2015-12-22 18:14:15 UTC
I tried your code on ppc 32-bit, with glibc 2.20 as I didn't have 2.22 available.

$ file bug
bug: ELF 32-bit MSB executable, PowerPC or cisco 4500, version 1 (SYSV), dynamically linked (uses shared libs), for GNU/Linux 2.6.32, not stripped

$ ./bug
before:
01111111111111111111111111111111111111111111111111...
after:
22222222222222222222222222222222201111111111111111...
the string should end here       ^
Comment 2 cvs268 2015-12-23 06:14:17 UTC
Tried the sample-code on ARM (Cortex A9) running Ubuntu 12.04.

gcc 4.6.3 complained about missing  stdalign.h.

So had to replace alignas(64) char s[144];
with __attribute__ ((aligned (64))) char s[144];
(hopefully the replacement works as intended.)

gcc test-strncat.c -std=c99 -o test-strncat

gcc --version
gcc (Ubuntu/Linaro 4.6.3-1ubuntu5) 4.6.3

file test-strncat
test-strncat: ELF 32-bit LSB executable, ARM, version 1 (SYSV), dynamically linked (uses shared libs), for GNU/Linux 2.6.31, BuildID[sha1]=0xb88137c1de1a1f534311ba81b1f403166b1091f1, not stripped

./test-strncat 
before:
01111111111111111111111111111111111111111111111111...
after:
22222222222222222222222222222222201111111111111111...
the string should end here       ^

So works properly, no issues, right?
Comment 3 Xavier Roche 2015-12-23 10:05:23 UTC
Possible duplicate of BUG 17279 (https://sourceware.org/bugzilla/show_bug.cgi?id=17279)
Comment 4 M. Simpson 2015-12-23 19:25:08 UTC
$ file a.out 
a.out: ELF 64-bit LSB executable, x86-64, version 1 (SYSV), dynamically linked, interpreter /lib64/ld-linux-x86-64.so.2, for GNU/Linux 2.6.32, BuildID[sha1]=8730e4750909db3ae281264c026e8ceb6ec4a140, not stripped

$ pacman -Q | grep glibc
glibc 2.22-3
lib32-glibc 2.22-3.1

$ ./a.out 
before:
01111111111111111111111111111111111111111111111111...
after:
22222222222222222222222222222222202222211111111111...
the string should end here       ^

This is on:
Arch Linux 4.2.5-1-ARCH #1 SMP PREEMPT Tue Oct 27 08:13:28 CET 2015 x86_64 GNU/Linux
Comment 5 Alexander Cherepanov 2015-12-26 19:09:08 UTC
Created attachment 8866 [details]
Test strncat for all offsets
Comment 6 Alexander Cherepanov 2015-12-26 19:14:51 UTC
(In reply to howey014 from comment #1)
> I tried your code on ppc 32-bit, with glibc 2.20 as I didn't have 2.22
> available.

(In reply to cvs268 from comment #2)
> Tried the sample-code on ARM (Cortex A9) running Ubuntu 12.04.
[skip]
> So works properly, no issues, right?

Thanks for testing it on different archs. Yes, this particular test is fine.

I attached a fuller test which tries all offsets and all lengths upto 64 (you can change it to 128 if you want). It will either print "Ok" or details of the first found fail. Please try it on your platforms.
Comment 7 Alexander Cherepanov 2015-12-26 19:24:27 UTC
(In reply to Xavier Roche from comment #3)
> Possible duplicate of BUG 17279
> (https://sourceware.org/bugzilla/show_bug.cgi?id=17279)

Yes, seems to be the same. So, this bug is not merely theoretical but is triggered by real code. Is your code open source / publicly available?
Comment 8 Adhemerval Zanella 2016-10-25 12:02:31 UTC
*** Bug 17279 has been marked as a duplicate of this bug. ***
Comment 9 cvs-commit@gcc.gnu.org 2017-01-03 16:25:58 UTC
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "GNU C Library master sources".

The branch, master has been updated
       via  8dad72997af2be0dc72a4bc7dbe82d85c90334fc (commit)
      from  d4d629e6187e33050902824a94498b6096eacac9 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=8dad72997af2be0dc72a4bc7dbe82d85c90334fc

commit 8dad72997af2be0dc72a4bc7dbe82d85c90334fc
Author: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Date:   Tue Jan 3 12:19:12 2017 -0200

    Fix x86 strncat optimized implementation for large sizes
    
    Similar to BZ#19387, BZ#21014, and BZ#20971, both x86 sse2 strncat
    optimized assembly implementations do not handle the size overflow
    correctly.
    
    The x86_64 one is in fact an issue with strcpy-sse2-unaligned, but
    that is triggered also with strncat optimized implementation.
    
    This patch uses a similar strategy used on 3daef2c8ee4df2, where
    saturared math is used for overflow case.
    
    Checked on x86_64-linux-gnu and i686-linux-gnu.  It fixes BZ #19390.
    
    	[BZ #19390]
    	* string/test-strncat.c (test_main): Add tests with SIZE_MAX as
    	maximum string size.
    	* sysdeps/i386/i686/multiarch/strcat-sse2.S (STRCAT): Avoid overflow
    	in pointer addition.
    	* sysdeps/x86_64/multiarch/strcpy-sse2-unaligned.S (STRCPY):
    	Likewise.

-----------------------------------------------------------------------

Summary of changes:
 ChangeLog                                        |   10 ++++++++++
 string/test-strncat.c                            |   15 +++++++++++++++
 sysdeps/i386/i686/multiarch/strcat-sse2.S        |    2 ++
 sysdeps/x86_64/multiarch/strcpy-sse2-unaligned.S |    2 ++
 4 files changed, 29 insertions(+), 0 deletions(-)
Comment 10 Adhemerval Zanella 2017-01-03 16:26:44 UTC
Fixed by 8dad72997af2be.