ntirpc icon indicating copy to clipboard operation
ntirpc copied to clipboard

There's some confusion about line 335 of clnt_dg.c

Open zhengyf opened this issue 6 years ago • 1 comments

https://github.com/nfs-ganesha/ntirpc/blob/dba123c3cbfa258923010a87a730618b8780b827/src/clnt_dg.c#L335

Let's see the code as follows

addr = (struct netbuf *)info;
if (addr->len < sizeof(cu->cu_raddr)) {
	rslt = false;
	break;

}
(void)memcpy(&cu->cu_raddr, addr->buf, addr->len);
cu->cu_rlen = addr->len;
break;

I think the condition of 'addr->len < sizeof(cu->cu_raddr)' is normal while the addr is for IPV4.

We should avoid the condition of 'addr->len > sizeof(cu->cu_raddr)' to prevent memory access out of bounds at '(void)memcpy(&cu->cu_raddr, addr->buf, addr->len);'

Is 'if (addr->len < sizeof(cu->cu_raddr)) ' supposed to be 'if (addr->len > sizeof(cu->cu_raddr)) ' ?

zhengyf avatar May 24 '18 08:05 zhengyf

No, the API here takes a struct sockaddr_storage, so that it can easily handle all address types. You're right that we should be checking the max length, so if anything the check should be (addr->len != sizeof(cu->raddr))

dang avatar May 24 '18 13:05 dang