[PATCH] [s3] CID 1433607, CID 1429799 and a possible mem leak

Swen Schillig swen at vnet.ibm.com
Fri May 25 09:01:32 UTC 2018


On Fri, 2018-05-25 at 10:42 +0200, Volker Lendecke via samba-technical
wrote:
> On Fri, May 25, 2018 at 10:20:41AM +0200, Swen Schillig via samba-
> technical wrote:
> > Please review and push if happy.
> 
> I'm not sure about the first one. This routine is just so spaghetti
> that I think it needs really thorough consideration. You might be
> right, but I've looked at that once and found it too complex code.
> 
> Volker
> 
Hmmm, maybe I overlook something but to me it looks like this

...
memset((char *)name,'\0',sizeof(*name));	#initialize entire 16-bytes to 0
...

while (m > 0) {
	unsigned char c1,c2;
	c1 = ubuf[offset++]-'A';
	c2 = ubuf[offset++]-'A';
	if ((c1 & 0xF0) || (c2 & 0xF0) || (n > sizeof(name->name)-1))
		return(0);
	name->name[n++] = (c1<<4) | c2;
	m -= 2;
}

... either 
	in while loop
		n > (16-1) then we return 0 from the if-clause
	outside while loop
	n > 16 => not possible
	n < 16 then the last byte we wrote was max name[14] and name[15) == 0 because of our memset
	n == 16 we enter....

if (n==MAX_NETBIOSNAME_LEN) {
	/* parse out the name type, its always
	 * in the 16th byte of the name */
	name->name_type = ((unsigned char)name->name[15]) & 0xff;

	/* remove trailing spaces */
	name->name[15] = 0;
	n = 14;
	while (n && name->name[n]==' ')
		name->name[n--] = 0;
}

...where everything will be handled accordingly.

Did I miss something ?

Cheers Swen




More information about the samba-technical mailing list