Bug in single stepping over a popf setting the trap flag

Title of the post nearly sums it up.  In win32, single stepping over a popf that sets the trap flag.  The trap flag when examined using GetThreadContext reports the trap flag as being clear.

I tried for the first time installing OllyDbg today also, but Olly has no problem in detecting the trap flag as set.  I’m not sure how it is able to do this.

To implement a solution in my own debugger, I will have to disassemble from the instruction pointer.  If its a popf, I will retrieve the contents from the stack and check for the trap flag being set.  If it is, I will call DbgContinue with DBG_EXCEPTION_NOT_HANDLED.

gdb leaves file descriptors open in debugee

I have my emulator running reasonably successfully on upx now.  It’s actually an auto unpacker, and identifies when the program is unpacked by monitoring execution on previously written memory.  In the process of emulating file io I came across a particular bug in gdb.

The file descriptor returned from an open call inside the debuggee, was 6.  I was expecting 3.

stdin=0, stdout=1,stderr=2

gdb must be using file descriptors 3,4,5, and forgot to close them before calling execve.

I’m not sure what the descriptors are used for.  Anyone care to take a look?

In the best case scenario, this bug can be used for another test to see if a debugger is present, and in the worst case if these file descriptors were used for control, *gasp* control gdb?  Probably they arent used for anything important, but I havent looked any furthur..

Setting sin_zero to 0 in struct sockaddr_in

I was looking at the Linux DHCP source code today which recently had a security advisory posted on it.   I looked at the patched version - without looking at the patch, and could not find any striking vulnerabilities.

I did however come across code that did something like this.

struct sockaddr_in sin;
sin.sin_family = PF_INET;
sin.sin_port = htons(PORT);
sin.sin_addr.s_addr = addr;
ret = sendto(…(struct sockaddr *)&sin…);

What it didn’t do was clear the sin_zero field in sockaddr_in to zero. This is a bug. I see it occur occasionaly.  This bug can cause undefined behaviour in applications.  In practice, I’ve seen functions such as sendto or recvfrom fail randomly because of lack of clearing sin_zero.  The intermittant nature of the error probably occured becuase I was using the stack for sockaddr_in, and often sockaddr_in was automatically set to zero, but occasionaly was not depending on the application in question.

 Firstly, what is sin_zero, and why should we care about it.


struct sockaddr_in
{
  sa_family_t    sin_family;   
  in_port_t      sin_port;     
  struct in_addr sin_addr;     

  unsigned char  __pad[__SOCK_SIZE__ - sizeof(short int)
                        - sizeof(unsigned short int) - sizeof(struct in_addr)];
};
#define sin_zero        __pad


That is from Windows cygwin /usr/include/cygwin/in.h.  The include file to use that is portable is /usr/include/in.h. 

Most of the net code does not use sockaddr_in, it uses sockaddr.  When you use a function like sendto, you must explicitly cast sockaddr_in, or whatever address your using, to sockaddr.  sockaddr_in is the same size as sockaddr, but internally the sizes are the same because of a slight hack.

 That hack is sin_zero.  Really the length of useful data in sockaddr_in is shorter than sockaddr.  But the difference is padded in sockaddr_in using a small buffer; that buffer is sin_zero. 

On some architectures, it wont cause any problems not clearing sin_zero.  But on other architectures it might.  Its required by specification to clear sin_zero, so you must do this if you intend your code to be bug free for now and in the future.

There are two ways of clearing sin_zero.


memset(&sin, 0, sizeof(sin));
sin.sin_family = …;
sin.sin_addr = …;
sin.sin_port = …;

or

sin.sin_family = …;
sin.sin_addr = …;
sin.sin_port = …;
memset(&sin.sin_zero, 0, sizeof(sin.sin_zero));

I prefer the later because in my view it performs slightly faster due to the fact that it doesnt set data to zero which then later gets reassigned.
I have seen people use a hardcoded figure of 8 for sizeof(sin.sin_zero). I believe this is also incorrect, and not as portable; for example, if a possible extention of sockaddr occurs in the future.  This probably will never occur, and some might argue that using 8 is ok, but in my opinion it is just nicer style.

configure ETCDIR buffer overflow

I just had a quick look at the source for webalyser, which is an opensource web log parser, and saw an interesting buffer overflow.  Before people get excited, its not a security problem or likely to ever be encountered.  It is presented here as an example of how bugs like this are still present in non critical code.

#define BUFSIZE 4096
…
char tmp_buf[BUFSIZE];
… 
sprintf(tmp_buf,”%s/webalizer.conf”, ETCDIR);

This is a classic buffer overflow that occurs when ETCDIR is configured to a size greater than BUFSIZE.  ETCDIR is user configured at buildtime via the configure script.

 In addition, when I first looked at main() there were about 20 variable declarations, and a mess of spaghetti would surely be to follow.  This was going to be a long night..

 The correct solution to avoid this buffer overflow would be to use snprintf.  snprintf might truncate the result however, so a better solution would be to dynamically allocate the memory required.


length = snprintf(NULL, 0, “%s/webalyser.conf”, ETCDIR);
tmp_buf = malloc(length + 1);
if (tmp_buf == NULL) error();
snprintf(tmp_buf, length, “%s/webalyser.conf”, ECTDIR);


The last snprintf could have been replaced with sprintf also, but I chose to use snprintf (read below).

The code above relies upon the behaviour of snprintf accepting NULL instead of a string argument and returning the number of bytes, excluding the trailing NUL, that would have been written to the string.  This behaviour is present from Linux glibc 2.1.  If you don’t have a system that implements this behaviour in snprintf, then you will be required to write a more complicated version of the code.


length = strlen(ETCDIR) + strlen(”/webalyser.conf”) + 1;
tmp_buf = malloc(length);
if (tmp_buf == NULL) error();
snprintf(tmp_buf, length, “%s/webalyser.conf”, ECTDIR);


The reason I chose to use snprintf and not sprintf, is because I believe that sprintf is inherently unsafe, and should not be used AT ALL - even when you know how to use it.  People might disagree with that view.

 A minor improvement might be to replace those strlen’s with sizeof.


length = sizeof(ETCDIR) + sizeof(”/webalyser.conf”) + 1;


In conclusion, I think many non system or performance critical programs written in legacy C have past their use by date.  If a modern language was used, be it either a scripting language or Java, perhaps even C#, then buffer overflows would be impossible to implement, and code readability generally improved.

Functions taking size_t and returning ssize_t

An interesting genre of bug occurs in C, when the return value and the success status of the function uses the same variable.

 Consider the read function.

 ssize_t read(int fd, void *buf, size_t count);

size_t is normally unsigned int, and ssize_t int.

 How big can count be?  The return value of read returns the number of bytes it read.  It should be equal to count when it succeeds (except in the case of a non blocking read).  But what if count is greater than INT_MAX?

 I’ve seen it to be the case that various functions in libc return (ssize_t)count, even when count is greater than INT_MAX.   This is the case with the 32bit lseek calls in Linux.

I am sure there are vulnerabilities present due to this behaviour.  Its only a question of where are they.

NULL argv[0] bugs in applications

An usual bug occurs when we force argv[0] to be NULL.  We can do this with execve functions.  Alot of userland code expects argv[0] to be the program name, and many programs will SEGV.  This is because alot of binaries do things as strrchr to extract the program name from the pathname.

When can a printf fail?

printf is one the fundamental libc functions, but when can it fail?  It can fail when the program output is being redirected to the disk, and the disk becomes full.

Does this mean we should check the return value of printf?  For absolutely robust code it would be nice, but I’ve never seen it adopted.  In production environments I have seen the *printf functions fail in glibc for unknown reasons under load.

The meaning of malloc returning NULL

NULL or (void *)0 as a return value for malloc means it has failed right?  Looking at the man page for malloc in Windows cygwin says that it means its failed.  But sometimes, malloc can reasonably return NULL even when it hasnt failed. 

 More portable and correct is setting errno.  Address 0, is a valid memory address.  I’ve never seen the heap put at 0, but no-one defines it as invalid.. except some unix implementations.

A quick google shows that not all systems implement this behaviour though.  Make sure to check the specifications for your particular system.
Better perhaps than malloc as an example, is mmap.  Sometimes I’ve wanted to return a mapping at address 0 for particular reasons.  Checking for errno is the only way gauranteed to give you the status. 

 As a developer I saw alot of bugs introduced by coders who assumed the return value and not errno was the definative source of success status.  You should always check the return value first, and if that indicates a possible failure, then check errno.

But then again, not all systems implement this (apparently).  How messy is ‘portable’ code to become if malloc has to be wrapped to check for errno, even if its correct?  For the record, I dont check errno on malloc ;-)  But on some systems, it is defined as being incorrect.  Not that this would ever be true in real life.

 I am, through all this, trying to make the point that when we treat a pointer as an address, then 0 is a valid response.  This is something we should keep in mind when working with pointers.

Linux Kernel Auditing; Unsigned Integer Overflows

For those that know me, you probably will know that for the past 5 years I have not been involved in computing. I did not even own a computer during that time! However, computing, even after 5 years is a hard thing to shake from the system, so I am starting to become involved again. I will be returning to University to study IT, and will soon, hopefully, own my own PC. Donations are always welcome

And so, the show must go on..

At the end of 2002 I performed audits of the operating system kernels in NetBSD, OpenBSD, FreeBSD and Linux with approximately 30-50 security patches introduced in the mainstream kernels as a result. The majority of vulnerable kernel code snippets are available as part of a 2003 archived presentation held at http://www.ruxcon.org.au.   A higher level review was presented in Las Vegas 2003, http://www.blackhat.com , also available online.

Last night I began an audit of the Linux kernel using the same methodology as in 2002. I will discuss this methodology in another blog. For now I will only discuss a particular class of C implementation bug that are still present in the Linux kernel.

Unsigned Integer overflows.These have been discussed for many years now, but were still present in a number of bugs discovered in the auditing I performed.

unsigned int a,b,c;
if (a+b > c)

This code is found in several places in the Linux Kernel, for checking exceeding rlimits. An rlimit or resource limit is a kernel limit of how many particular operating systems resources can be used by a particular user, or how much of a resource can be used. These vulnerabilities in Linux allows some resource limits to be evaded.

“a” and “b” in the Linux kernel code were not truely limits in their own right, but the cumulative total represented a resource limit. They were both unsigned in type. An integer overflow occurred when either “a” or “b” was large, making the total overflow.

Related to these vulnerabilities are the cases when signed integers overflow in an operation, or unsigned and signed integers overflow.

Other details include overflows involving integers of different sizes, and architectures where integer types are of different size.

Also signifcant to this, is type promotion, which makes vulnerable some otherwise sound code.

These problems are slightly different in semantics, and will not be discussed any furthur.

Back to what we were talking about..

As an example in the case where an unsigned integer type is 8 bits in size, 0×01 + 0xff, would overflow. The correct answer to the addition 0×100, but with only 8 bits available to store the result, the high bit is truncated and the real result becomes 0×00.

In the Linux example described earlier (a+b>c), a better (but still incorrect) implementation of vulnerable code might look like this

unsigned int a,b,c;
if (a > c || b > c || (a+b)>c)

I have seen this type of code implemented to check for these types of problems, in all the kernels I audited, espceially NetBSD. However, it is still vulnerable. An example in 8 bits is when a=0×80, b=0×80, both less than c=0×90. An integer overflow occurs in 0×80+0×80 resulting in 0×100. An explicit check for an overflow should occur.

unsigned int a,b,c;
if ((a+b)>c || (a+b) < a)

A typical correct implementation in C to check for overflow is demonstrated above. When “c=a+b” overflows, then c is always less than either “a” or “b”. In the case above checking against “a” was performed, although the choice could have been “b” also.

That concludes the story on this one class of integer overflow commonly seen in vulnerable code; in our case, the Linux kernel. I will discuss more bug classes and vulnerabilities in future posts.