OSEC

Neohapsis is currently accepting applications for employment. For more information, please visit our website www.neohapsis.com or email hr@neohapsis.com
 
Subject: Things to look out for when auditing
From: Chris Evans (chrisferret.lmh.ox.ac.uk)
Date: Wed Jun 07 2000 - 19:46:24 CDT

Hey,

Just a few pointers on some fairly common classes of code flaw I've been
researching/hitting recently.

Anyone doing an audit might want to look out for these. Anyone writing new
code which has to be secure, would do well to bear these in mind.

The main flaw is integer overflow (or underflow). The problem occurs in
several guises. Let's have a quick look at some.

1) Integer overflow when allocating arrays of object

unsigned int nItems = get_u32_from_network();
unsigned int size = nItem * sizeof(some_object);
// Optionally check size against an upper bound, we can get past that
char* mem = malloc(size);
// Now read into mem. We're in trouble if we use units of "some_object"

In the above code snippet, if we set nItem to UINT_MAX then we end up with
"size" set to a small positive integer. We may now be heading for a
dangerous malloc()'ed buffer overflow.

2) Integer overflow by adding "header" object to a code

unsigned int size = get_u32_from_network();
char* mem = malloc(nItems + sizeof(some_hdr));
// Now read into mem

So, by setting nItem close to UINT_MAC, we end up with a small positive
integer with the same dangerous malloc() overflow potentials.

3) Wrapping virtual address space to break buffer bounds

char buf[1000];
char* cp = buf;

unsigned int size = get_u32_from_network();
cp += size; // skip something
if (cp > buf + sizeof(buf))
  goto overflow;

Primarily applying to architechtures with 32 bit address space, the above
code sample is flawed. If cp==buf, and we read in a size of UINT_MAX, our
end pointer is cp==(buf-1), because we wrap virutal address space. We pass
the overflow sanity check by ending up before the start of the buffer
instead of beyond it. We have violated the bounds of the buffer.

4) signed/unsigned conversions

char buf[1000];
int size = get_u32_from_network();

if (size > sizeof(buf))
  goto out;
// Oops: read into buf

Above, we set size to "-1" and pass the security sanity check. "-1" is
liable to be treated as 4Gb by some subsequent memcpy() or read
operation. Very dangerous, we'll break the bounds of buf.

GENERAL
=======

If you encounter a network protocol which includes explicit user provided
lengths as part of the protocol, then watch out! _Especially_ if these
values are defined as _32 bit quantities in the protocol_. The opportunity
for wrapping/overflow is a lot worse with 32 bit quantities. Even a lot of
64 bit machines have 32 bit "int".

When auditing code, always be on the lookout to supply extreme values to
any size or count variable. Extreme means things like UINT_MAX (or 16 bit
max or 8 bit char max). Try -1, it's often a killer. Some programs get
into trouble when an unexpected 0 is encountered. If you have a packet
based protocol and each packet has a length, try a short total packet
length but a large count or size field within the packet.

Just some food for thought

Cheers
Chris