bug in smb_io_dom_sid()

[Andrew Tridgell]

Luke,

smbd crashes for me in api_lsa_lookup_sids(). From netmon traces it
looks like the bug is in smb_io_dom_sid() called from
lsa_io_sid_enum() via smb_io_dom_sid2(). In particular the following
part of the packet doesn't exist:

	prs_uint32s(False, "sub_auths ", ps, depth, sid->sub_auths, sid->num_auths);

and the above bit:

	prs_uint8 ("num_auths  ", ps, depth, &(sid->num_auths));

	for (i = 0; i < 6; i++)
	{
		fstring tmp;
		slprintf(tmp, sizeof(tmp) - 1, "id_auth[%d] ", i);
		prs_uint8 (tmp, ps, depth, &(sid->id_auth[i]));
	}

looks wrong. num_auths is 6, so maybe the loop should be until
num_auths?

what happens is that the prs_uint32s() call chews up bytes from the
tail of the packet so that when lsa_io_q_lookup_sids() goes to call
lsa_io_trans_names() the data pointer is pointing too far into the
packet and you get garbage for num_entries2, and thus a segfault. This
was complicated by the lack of structure initialisation that I
mentioned earlier, but the lack of initialisation wasn't the primary
problem. 

I'll leave it to you to decide if we need a smb_io_dom_sid4() or need
to fix smb_io_dom_sid() in all cases. 

Whenever I look at this stuff I am in awe of the fact that you got it
to work at all but I am in agony over how fragile the whole thing
is. The fragility is probably due to the structure of the protocol
more than anything else, but I think to survive we are going to need
_liberal_ use of ASSERT(). So for example we could have:

ASSERT(trn->num_entries2 <= MAX_LOOKUP_SIDS);

and have:

#define ASSERT(x) ((x)?0:panic("assertion failed at %s(%d)\n",__FILE__, __LINE__))

we need this in heaps of places so we have some chance of tracking
down these problems. Then we need panic() to allow us to attach a
debugger. 

see ftp://samba.anu.edu.au/pub/tridge/misc/lsalookup.cap and look at
frame 221 for the above problem. note that in frame 191 the same call
is made and it "works". In frame 221 a debugger on the dead process
shows that trn->num_entries2 is 8257538 (ie. 0x7E0002) which comes
from some stuff later in the packet. Also, the following is clearly
garbage: 

(gdb) p q_s->sids->sid.sid.sub_auths
$14 = {21, 123, 456, 76, 42, 1416, 0, 0, 0, 0, 0, 0, 0, 0, 0}

reproduce it under NT4ws by right clicking on the control panel,
selecting properties then clicking on "user profiles" when your
profile is stored on the samba server. Produces segv death most times
for me (sometimes the random garbage means it gets away with it!)