Chromium Code Reviews Chromium Code Reviews
Issue
12079010:
Avoid undefined behavior when checking for pointer wraparound (Closed)
DescriptionAvoid undefined behavior when checking for pointer wraparound due to
oversized pickled payload. Since pointer wraparound is undefined
behavior in C, the current code in Pickle::FindNext():
const char* payload_base = start + header_size;
const char* payload_end = payload_base + hdr->payload_size;
if (payload_end < payload_base)
return NULL;
which is equivalent to:
if (start + header_size + hdr->payload_size < start + header_size)
return NULL;
can be simplified by some compilers (e.g., clang) to something like:
if (header_size + hdr->payload_size < header_size)
return NULL;
For example, here's a simplified code snippet and the generated code,
which doesn't access the pointer argument at all:
nickolai@sahara:/tmp$ cat x.c
#include <stdio.h>
#include <stdint.h>
#include <sys/types.h>
void bar();
void
foo(const char* start, size_t header_size, uint32_t payload_size)
{
const char* payload_base = start + header_size;
const char* payload_end = payload_base + payload_size;
if (payload_end < payload_base)
bar();
}
nickolai@sahara:/tmp$ clang x.c -S -o - -O3 -m32
...
foo: # @foo
# BB#0:
movl 8(%esp), %eax
movl 12(%esp), %ecx
addl %eax, %ecx
cmpl %eax, %ecx
jge .LBB0_1
# BB#2:
jmp bar # TAILCALL
.LBB0_1:
ret
...
nickolai@sahara:/tmp$
Thus, on a 32-bit machine, this can lead to oversized pickle headers
bypassing this check and causing undetected pointer wraparound; the caller
may then be tricked into accessing memory outside of the pickle payload.
The patch replaces the check with one that avoids undefined behavior,
and fixes a similar problem in PickleIterator::GetReadPointerAndAdvance
pointed out by jar@chromium.org.
BUG=none
Patch Set 1 #Patch Set 2 : #
Total comments: 2
Patch Set 3 : #
Total comments: 2
Patch Set 4 : #Patch Set 5 : #Patch Set 6 : #Patch Set 7 : #Patch Set 8 : #Messages
Total messages: 30 (0 generated)
|
