25 May
2016
25 May
'16
11:06 p.m.
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.
Thanks,
Lucian
[0]https://github.com/wbx-github/uclibc-ng/blob/master/libc/string/arm/memse…
[1]http://tomato.groov.pl/download/K26ARM/132/tomato-R7000-ARM--132-AIO-64K.zip
[2]disas.S (attached)
[3]qemu.log (attached)
[4]http://www.cvedetails.com/cve/CVE-2011-2702/
[5]http://old.sebug.net/paper/Exploits-Archives/2012-exploits/1208-exploits/…
25 May
25 May
11:45 p.m.
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.).
Rich
26 May
26 May
9:07 a.m.
New subject: [Spam] Re: uClibc-ng and uClibc memset bug, ARM
On 05/26/2016 01:45 AM, Rich Felker wrote:
On Thu, May 26, 2016 at 01:06:40AM +0200, Lucian
Cojocar wrote:
Is this defined by some standard (i.e. objects should be no larger than
than PTRDIFF_MAX)?
Lucian
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.).
7:34 p.m.
New subject: [Spam] Re: uClibc-ng and uClibc memset bug, ARM
On Thu, May 26, 2016 at 11:07:33AM +0200, Lucian Cojocar wrote:
On 05/26/2016 01:45 AM, Rich Felker wrote:
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 Thu, May 26, 2016 at 01:06:40AM +0200, Lucian
Cojocar wrote:
Is this defined by some standard (i.e. objects should be no larger than
than PTRDIFF_MAX)? 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.).
27 May
27 May
6:32 p.m.
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?
best regards
Waldemar
30 May
30 May
8:57 a.m.
On 05/27/2016 08:32 PM, Waldemar Brodkorb wrote:
Hi Lucian,
Lucian Cojocar wrote,
I'll propose a patch before mid-next week.
Lucian
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?
10 Jun
10 Jun
4:44 p.m.
New subject: [PATCH] bugfix: ARM: memset.S: use unsigned comparisons
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.
memset(buf, 0xaa, 0xffff0000) triggers the bug.
GDB session without the patch:
"""
$ gdb ./main-buggy-memset.elf -q
Reading symbols from ./main-buggy-memset.elf...done.
(gdb) x/i memset
0x8770 <memset>: mov r3, r0
(gdb) r
Starting program: /root/memset/main-buggy-memset.elf
Program received signal SIGSEGV, Segmentation fault.
0x00048808 in ?? ()
"""
The $pc is outside of the memset function because:
"""
(gdb) x/i $pc
=> 0x87e4 <memset+116>: add pc, pc, r2, lsl #2
(gdb) info reg $r2
r2 0x10007 65543
"""
GDB session with the bug fixed (patch applied):
"""
$ gdb ./main-fixed-memset.elf -q
Reading symbols from ./main-fixed-memset.elf...done.
(gdb) x/i memset
0x8770 <memset>: mov r3, r0
(gdb) r
Starting program: /root/memset/main-fixed-memset.elf
Program received signal SIGSEGV, Segmentation fault.
memset () at libc/string/arm/memset.S:92
92 libc/string/arm/memset.S: No such file or directory.
(gdb) x/i $pc
=> 0x87b0 <memset+64>: stmia r3!, {r1, r12}
(gdb) info reg $r3
r3 0x15000 86016
(gdb) info proc mappings
process 5822
Mapped address spaces:
Start Addr End Addr Size Offset objfile
0x8000 0xb000 0x3000 0x0
/root/memset/main-fixed-memset.elf
0x12000 0x15000 0x3000 0x2000
/root/memset/main-fixed-memset.elf
0xb6fff000 0xb7000000 0x1000 0x0 [sigpage]
0xbefdf000 0xbf000000 0x21000 0x0
0xffff0000 0xffff1000 0x1000 0x0 [vectors]
(gdb) info reg $sp
sp 0x14d78 0x14d78
"""
GDB crashes inside the memset function, on the store instruction. This
time the crash is (as expected) because of a memory access imediately
after the memory region that contains the stack -- the buffer that's
being memset'd is allocated on the stack.
Signed-off-by: Lucian Cojocar <lucian.cojocar(a)vu.nl>
---
libc/string/arm/memset.S | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/libc/string/arm/memset.S b/libc/string/arm/memset.S
index 2be4850..412270f 100644
--- a/libc/string/arm/memset.S
+++ b/libc/string/arm/memset.S
@@ -67,7 +67,7 @@ memset:
memset:
mov a4, a1
cmp a3, $8 @ at least 8 bytes to do?
- blt 2f
+ blo 2f
orr a2, a2, a2, lsl $8
orr a2, a2, a2, lsl $16
1:
@@ -84,27 +84,27 @@ memset:
mov ip, a2
1:
cmp a3, $8 @ 8 bytes still to do?
- blt 2f
+ blo 2f
stmia a4!, {a2, ip}
sub a3, a3, $8
cmp a3, $8 @ 8 bytes still to do?
- blt 2f
+ blo 2f
stmia a4!, {a2, ip}
sub a3, a3, $8
cmp a3, $8 @ 8 bytes still to do?
- blt 2f
+ blo 2f
stmia a4!, {a2, ip}
sub a3, a3, $8
cmp a3, $8 @ 8 bytes still to do?
#if defined(__thumb2__)
- itt ge
- stmiage a4!, {a2, ip}
- subge a3, a3, $8
+ itt hs
+ stmiahs a4!, {a2, ip}
+ subhs a3, a3, $8
#else
- stmgeia a4!, {a2, ip}
- subge a3, a3, $8
+ stmhsia a4!, {a2, ip}
+ subhs a3, a3, $8
#endif
- bge 1b
+ bhs 1b
2:
movs a3, a3 @ anything left?
IT(t, eq)
--
2.1.4
19 Jun
19 Jun
6:31 p.m.
New subject: [PATCH] bugfix: ARM: memset.S: use unsigned comparisons
Hi,
Any follow-up on this patch?
Thanks,
Lucian
On 06/10/2016 06:44 PM, Lucian Cojocar wrote:
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.
memset(buf, 0xaa, 0xffff0000) triggers the bug.
GDB session without the patch:
"""
$ gdb ./main-buggy-memset.elf -q
Reading symbols from ./main-buggy-memset.elf...done.
(gdb) x/i memset
0x8770 <memset>: mov r3, r0
(gdb) r
Starting program: /root/memset/main-buggy-memset.elf
Program received signal SIGSEGV, Segmentation fault.
0x00048808 in ?? ()
"""
The $pc is outside of the memset function because:
"""
(gdb) x/i $pc
=> 0x87e4 <memset+116>: add pc, pc, r2, lsl #2
(gdb) info reg $r2
r2 0x10007 65543
"""
GDB session with the bug fixed (patch applied):
"""
$ gdb ./main-fixed-memset.elf -q
Reading symbols from ./main-fixed-memset.elf...done.
(gdb) x/i memset
0x8770 <memset>: mov r3, r0
(gdb) r
Starting program: /root/memset/main-fixed-memset.elf
Program received signal SIGSEGV, Segmentation fault.
memset () at libc/string/arm/memset.S:92
92 libc/string/arm/memset.S: No such file or directory.
(gdb) x/i $pc
=> 0x87b0 <memset+64>: stmia r3!, {r1, r12}
(gdb) info reg $r3
r3 0x15000 86016
(gdb) info proc mappings
process 5822
Mapped address spaces:
Start Addr End Addr Size Offset objfile
0x8000 0xb000 0x3000 0x0
/root/memset/main-fixed-memset.elf
0x12000 0x15000 0x3000 0x2000
/root/memset/main-fixed-memset.elf
0xb6fff000 0xb7000000 0x1000 0x0 [sigpage]
0xbefdf000 0xbf000000 0x21000 0x0
0xffff0000 0xffff1000 0x1000 0x0 [vectors]
(gdb) info reg $sp
sp 0x14d78 0x14d78
"""
GDB crashes inside the memset function, on the store instruction. This
time the crash is (as expected) because of a memory access imediately
after the memory region that contains the stack -- the buffer that's
being memset'd is allocated on the stack.
Signed-off-by: Lucian Cojocar <lucian.cojocar(a)vu.nl>
---
libc/string/arm/memset.S | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/libc/string/arm/memset.S b/libc/string/arm/memset.S
index 2be4850..412270f 100644
--- a/libc/string/arm/memset.S
+++ b/libc/string/arm/memset.S
@@ -67,7 +67,7 @@ memset:
memset:
mov a4, a1
cmp a3, $8 @ at least 8 bytes to do?
- blt 2f
+ blo 2f
orr a2, a2, a2, lsl $8
orr a2, a2, a2, lsl $16
1:
@@ -84,27 +84,27 @@ memset:
mov ip, a2
1:
cmp a3, $8 @ 8 bytes still to do?
- blt 2f
+ blo 2f
stmia a4!, {a2, ip}
sub a3, a3, $8
cmp a3, $8 @ 8 bytes still to do?
- blt 2f
+ blo 2f
stmia a4!, {a2, ip}
sub a3, a3, $8
cmp a3, $8 @ 8 bytes still to do?
- blt 2f
+ blo 2f
stmia a4!, {a2, ip}
sub a3, a3, $8
cmp a3, $8 @ 8 bytes still to do?
#if defined(__thumb2__)
- itt ge
- stmiage a4!, {a2, ip}
- subge a3, a3, $8
+ itt hs
+ stmiahs a4!, {a2, ip}
+ subhs a3, a3, $8
#else
- stmgeia a4!, {a2, ip}
- subge a3, a3, $8
+ stmhsia a4!, {a2, ip}
+ subhs a3, a3, $8
#endif
- bge 1b
+ bhs 1b
2:
movs a3, a3 @ anything left?
IT(t, eq)
6:40 p.m.
New subject: [PATCH] bugfix: ARM: memset.S: use unsigned comparisons
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?
Or do you have a simple testcase showing breakage before your patch
and non-breakage after?
best regards
Waldemar
9:20 p.m.
New subject: [PATCH] bugfix: ARM: memset.S: use unsigned comparisons
Waldemar Brodkorb <wbx <at> ucibc-ng.org> writes:
Hi Lucian,
Lucian Cojocar wrote,
'memset' will work on a range >= 2GB.
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? 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
26 Jun
26 Jun
10:23 a.m.
New subject: [PATCH] bugfix: ARM: memset.S: use unsigned comparisons
Hi Lucian,
Lucian Cojocar wrote,
Waldemar Brodkorb <wbx <at> ucibc-ng.org>
writes:
Applied and pushed,
Thanks you,
Waldemar
Hi Lucian,
Lucian Cojocar wrote,
'memset' will work on a range >= 2GB.
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? 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
3071
days inactive
3103
days old
10 comments
4 participants
participants (4)
-
Lucian Cojocar -
Rich Felker -
Waldemar Brodkorb -
Waldemar Brodkorb