[9fans] dial and buffer overflow

[9fans] dial and buffer overflow

[Mathieu L.]

Hello all,

I have this piece of code that looks like this:

	for (int i=0; i<5 ; i++){
		for (int j=0; j<HASHSIZE; j++)
			print("%.2ux", (tor->sha1list)[i][j]);
		print("\n");
	}

	print("addr called: %s\n",netmkaddr(address, "tcp", port));
	ctlfd = dial(netmkaddr(address, "tcp", port), 0, 0, 0);
	if(ctlfd < 0){
		fprint(2, "can't dial %s: %r\n", address);
		exits("dialing");
	}

	for (int i=0; i<5 ; i++){
		for (int j=0; j<HASHSIZE; j++)
			print("%.2ux", (tor->sha1list)[i][j]);
		print("\n");
	}

which gives that kind of input:

f8c3f943edf54d28e3f894e9416d5312a49c3916
5d7a30beaef2b56a06b8aea37cd3263698825ec3
b456f6749bf907233c183c04277569aa0833e386
3e2d9cee1e07d3d770f1a6081a006394cb4b35d3
c43a3bd4caa813a75f58096068309ede6e96cacd
addr called: tcp!127.0.0.1!6895
6970000034930300eb9803000e0000000e000000
5d7a30bed80802313cc70000349303003ffd0100
b456f6749bf907233c183c04277569aa0833e386
3e2d9cee1e07d3d770f1a6081a006394cb4b35d3
c43a3bd4caa813a75f58096068309ede6e96cacd

so it seems like something happens when calling dial which modifies what
I have in memory pointed by tor->sha1list[i], and that is of course not
what I want, as those values should have nothing to do with the call to
dial itself.

the various tor->sha1list[i][j] were allocated and set earlier in the
code, and I don't think I wrote out of their boundaries when I did so
since I can free() them all just before the call to dial() without getting
an error. Although I'm not sure that's a conclusive enough test, is it?

It's on 9vx so I can't use acid or leak since both systematically freeze
9vx here.

Does anyone have an idea on what I'm doing wrong or how to debug that
further?

Thanks,
Mathieu

Re: [9fans] dial and buffer overflow

[erik quanstrom]

[-- Attachment #1: Type: text/plain, Size: 2146 bytes --]

On Sat Jul  4 11:45:50 EDT 2009, lejatorn@gmail.com wrote:
> Hello all,
>
> I have this piece of code that looks like this:
>
> 	for (int i=0; i<5 ; i++){
> 		for (int j=0; j<HASHSIZE; j++)
> 			print("%.2ux", (tor->sha1list)[i][j]);
> 		print("\n");
[...]
> which gives that kind of input:
>
> f8c3f943edf54d28e3f894e9416d5312a49c3916
> 5d7a30beaef2b56a06b8aea37cd3263698825ec3
> b456f6749bf907233c183c04277569aa0833e386
> 3e2d9cee1e07d3d770f1a6081a006394cb4b35d3
> c43a3bd4caa813a75f58096068309ede6e96cacd
> addr called: tcp!127.0.0.1!6895
> 6970000034930300eb9803000e0000000e000000
> 5d7a30bed80802313cc70000349303003ffd0100
> b456f6749bf907233c183c04277569aa0833e386
> 3e2d9cee1e07d3d770f1a6081a006394cb4b35d3
> c43a3bd4caa813a75f58096068309ede6e96cacd
[...]
> so it seems like something happens when calling dial which modifies what
> I have in memory pointed by tor->sha1list[i],

the extra () around tor->sha1list are confusing.

it is more likely that you have some allocation
error in your code.  neither netmkaddr nor
dial do any allocation, so i don't see how memory
on the heap could get corrupted without help.

the three most common errors that cause this are
(a) not allocating enough memory by, e.g.
malloc(sizeof tor) not malloc(sizeof *tor). or
(b) pointing to a non-static on the stack,
or (c) mistyping of tor->sha1list so that sizeof
tor->sha1list[0][0] != 1.

i've attached a filled-out version that has no
memory leaks as verified by leak and produces
correct output.  if you wish you can check
pool(2) and set up all kinds of pool checking.
to verify.

here's the output
; 8c -FVTw l.c && 8l l.8 && 8.out
000102030405060708090a0b0c0d0e0f10111213
1415161718191a1b1c1d1e1f2021222324252627
28292a2b2c2d2e2f303132333435363738393a3b
3c3d3e3f404142434445464748494a4b4c4d4e4f
505152535455565758595a5b5c5d5e5f60616263
addr called: tcp!ladd!8088
pid 376527
000102030405060708090a0b0c0d0e0f10111213
1415161718191a1b1c1d1e1f2021222324252627
28292a2b2c2d2e2f303132333435363738393a3b
3c3d3e3f404142434445464748494a4b4c4d4e4f
505152535455565758595a5b5c5d5e5f60616263

- erik

[-- Attachment #2: l.c --]
[-- Type: text/plain, Size: 1037 bytes --]

#include <u.h>
#include <libc.h>
#include <libsec.h>

enum {
	Nsha	= 5,
};

typedef struct Tor Tor;
struct Tor{
	uchar	sha1list[Nsha][SHA1dlen];
};

void
sha1pr(uchar *u)
{
	char buf[SHA1dlen*2 + 1];
	int i;

	for(i = 0; i < SHA1dlen; i++)
		sprint(buf + 2*i, "%.2ux", u[i]);
	print("%s\n", buf);
}

void
main(void)
{
	char *addr, *port;
	int i, j, fd;
	Tor *tor;

	port = strdup("8088");
	addr = strdup("ladd");
	tor = malloc(sizeof *tor);
	if(tor == nil)
		sysfatal("malloc: %r");
	memset(tor, 0, sizeof *tor);

	for(i = 0; i < Nsha; i++)
		for(j = 0; j < SHA1dlen; j++)
			tor->sha1list[i][j] = i*20 + j;

	for(i = 0; i < Nsha ; i++)
		sha1pr(tor->sha1list[i]);
	print("addr called: %s\n", netmkaddr(addr, "tcp", port));
	fd = dial(netmkaddr(addr, "tcp", port), nil, nil, nil);
	if(fd < 0){
		fprint(2, "can't dial %s: %r\n", addr);
		exits("dialing");
	}
	close(fd);

	for(i = 0; i < Nsha; i++)
		sha1pr(tor->sha1list[i]);
	free(tor);
	free(addr);
	free(port);
	exits("");
};

Re: [9fans] dial and buffer overflow

[Mathieu L.]

Thanks for the answer Erik.

[...]
> the extra () around tor->sha1list are confusing.

Noted, thanks, they're gone. I suck at remembering operators precedence
so I usually add a few parentheses to be on the safe side.

> it is more likely that you have some allocation
> error in your code.  neither netmkaddr nor
> dial do any allocation, so i don't see how memory
> on the heap could get corrupted without help.
>
> the three most common errors that cause this are
> (a) not allocating enough memory by, e.g.
> malloc(sizeof tor) not malloc(sizeof *tor). or
> (b) pointing to a non-static on the stack,
> or (c) mistyping of tor->sha1list so that sizeof
> tor->sha1list[0][0] != 1.

Could you elaborate on (b), or point me to an example please? I don't
really understand what it means or how it could happen...

Re: [9fans] dial and buffer overflow

[erik quanstrom]

On Sat Jul  4 18:12:16 EDT 2009, lejatorn@gmail.com wrote:
> Thanks for the answer Erik.
>
> [...]
> > the extra () around tor->sha1list are confusing.
>
> Noted, thanks, they're gone. I suck at remembering operators precedence
> so I usually add a few parentheses to be on the safe side.

i think it's important to learn operator precedence.
why guess when you can know?  you can steal
the cheet sheet i stole from bwc:

#! /bin/rc
label prec
cat <<!
left to right	() [] -> .
right to left	! ~ ++ -- - (type) * & sizeof
left to right	* / %
left to right	+ -
left to right	<< >>
left to right	< <= > >=
left to right	== !=
left to right	&
left to right	^
left to right	|
left to right	&&
left to right	||
right to left	?:
right to left	= += -= etc.
left to right	,
!

> > the three most common errors that cause this are
> > (a) not allocating enough memory by, e.g.
> > malloc(sizeof tor) not malloc(sizeof *tor). or
> > (b) pointing to a non-static on the stack,
> > or (c) mistyping of tor->sha1list so that sizeof
> > tor->sha1list[0][0] != 1.
>
> Could you elaborate on (b), or point me to an example please? I don't
> really understand what it means or how it could happen...

generally, it's not so obvious, but a variation of this
would do it:

uchar*
badfn(char *txt, uint len)
{
	uchar buf[SHA1dlen];

	sha1(txt, len, sha, nil);
	return buf;
}

- erik