On Thu, May 26, 2016 at 11:07:33AM +0200, Lucian Cojocar wrote:

On 05/26/2016 01:45 AM, Rich Felker wrote:

...

On Thu, May 26, 2016 at 01:06:40AM +0200, Lucian Cojocar wrote:

...

Hi all,

in libc/string/arm/memset.S[0]. If the code is compiled with #undef __thumb2__ and with #undef THUMB1_ONLY (this seems to be case for Tomato[1] at least and for buildroot) then the code looks like this[2]:

""" memset: mov a4, a1 cmp a3, $8 @ at least 8 bytes to do? blt 2f orr a2, a2, a2, lsl $8 orr a2, a2, a2, lsl $16 ... 2: movs a3, a3 @ anything left? IT(t, eq) BXC(eq, lr) @ nope

rsb a3, a3, $7 add pc, pc, a3, lsl $2 <--- a3 can be larger than $7 here mov r0, r0 strb a2, [a4], $1 strb a2, [a4], $1 ... """"

The problem is that the 'BLT' instruction checks for *signed* values. So if a3, length parameter of memset, is negative, then value added to the PC will be large.

In short, an attacker gains control of PC through the len parameter of memset. The attack is a bit unrealistic, as it requires that the application that uses uClibc allows a user to control a memory chunk larger than 2GB.

I only tested this on qemu-system-arm[3]. The code was just calling memset(buf, 0xaa, 0xffff0000), memset, in this example[3] is @0x1003c.

This bug is similar to CVE-2011-2702[4, 5]. Probably we should notify oss-security and get a CVE for this as the impact is unknown.

This is only one of a HUGE number of things that go hopelessly wrong if an implementation allows single objects with sizes larger than PTRDIFF_MAX. A lot has been written on this topic recently. Regardless of how this one report is resolved, uClibc should ensure that no such objects can arise (by checking sizes in malloc, mmap, etc.).

Is this defined by some standard (i.e. objects should be no larger than than PTRDIFF_MAX)?

No; but it's very hard to make an implementation that supports such objects and conforms to other requirements of the standard. All existing compilers I'm aware of have numerous serious problems with trying to support such objets. It's also very hard for application code to safely use implementations that support such objects; it becomes unsafe to subtract pointers unless you know ahead of time that the difference won't overflow, since pointer differences that overflow produce undefined behavior.

As I said, a lot has been written on this topic recently -- GCC and LLVM bug reports, blog posts, papers, twitter threads, etc. The details are way too big for me to cover extensively in an email reply, but the information is out there if you search for it.

Rich

On 05/27/2016 08:32 PM, Waldemar Brodkorb wrote:

Hi Lucian, Lucian Cojocar wrote,

...

On 05/26/2016 01:45 AM, Rich Felker wrote:

...

On Thu, May 26, 2016 at 01:06:40AM +0200, Lucian Cojocar wrote:

...

Hi all,

in libc/string/arm/memset.S[0]. If the code is compiled with #undef __thumb2__ and with #undef THUMB1_ONLY (this seems to be case for Tomato[1] at least and for buildroot) then the code looks like this[2]:

The problem is that the 'BLT' instruction checks for *signed* values. So if a3, length parameter of memset, is negative, then value added to the PC will be large.

In short, an attacker gains control of PC through the len parameter of memset. The attack is a bit unrealistic, as it requires that the application that uses uClibc allows a user to control a memory chunk larger than 2GB.

I only tested this on qemu-system-arm[3]. The code was just calling memset(buf, 0xaa, 0xffff0000), memset, in this example[3] is @0x1003c.

This bug is similar to CVE-2011-2702[4, 5]. Probably we should notify oss-security and get a CVE for this as the impact is unknown.

This is only one of a HUGE number of things that go hopelessly wrong if an implementation allows single objects with sizes larger than PTRDIFF_MAX. A lot has been written on this topic recently. Regardless of how this one report is resolved, uClibc should ensure that no such objects can arise (by checking sizes in malloc, mmap, etc.).

Will you propose a patch?

I'll propose a patch before mid-next week.

Lucian

Waldemar Brodkorb <wbx <at> ucibc-ng.org> writes:

Hi Lucian, Lucian Cojocar wrote,

...

Hi,

Any follow-up on this patch?

Not yet. You are saying the second segmentation fault would be expected. What is then the exact benefit of the patch, if the result is a segfault?

'memset' will work on a range >= 2GB.

Or do you have a simple testcase showing breakage before your patch and non-breakage after?

Yes, I have a test which works in an environment where you can use more than 2GB contiguous virtual memory (e.g. have enough swap to accommodate 2GB contiguous virtual memory).

""" #include <string.h> #include <stdio.h> #include <sys/mman.h>

#define GB (size_t)(1*(1 << 30ul)) #define TWO_GB (2*(size_t)(GB)) #define S (size_t)(TWO_GB+4096)

int main(void) { char *p = NULL; p = mmap(NULL, S, PROT_READ|PROT_WRITE, MAP_ANONYMOUS|MAP_PRIVATE, -1, 0); if (p == MAP_FAILED) { perror("mmap"); exit(-1); } printf("&p[0] = %p\n", &p[0]); printf("&p[S] = %p\n", &p[S]);

printf("memsetting ..."); memset(p, 0xaa, S); printf("done\n");

printf("p[S-1]=%02x\n", (unsigned char)p[S-1]); exit(0); } """

~# ./main-buggy-memset.elf &p[0] = 0x36f66000 &p[S] = 0xb6f67000 Segmentation fault ~# ./main-fixed-memset.elf &p[0] = 0x36f8a000 &p[S] = 0xb6f8b000 memsetting ...done p[S-1]=aa ~# free -h total used free shared buffers cached Mem: 247M 56M 191M 252K 4.3M 33M -/+ buffers/cache: 19M 228M Swap: 10G 16M 10G