DHCP decoder handling of header string fields

[Chaigneau, Nicolas]

Hello,


I've noticed that FreeRADIUS DHCP decoder is handling strings from the DHCP header fields a bit oddly.

Basically, it allocates a value pair buffer with the maximum field length (64 or 128 octets, for DHCP-Server-Host-Name and DHCP-Boot-Filename respectively) + 1, and memcpy the data then add a '\0'.

But RFC 2131 explicitly states that these DHCP fields are null terminated strings. So why not handle them as such ?




Current code from "fr_dhcp_decode" function:

		case PW_TYPE_STRING:
			q = talloc_array(vp, char, dhcp_header_sizes[i] + 1);
			memcpy(q, p, dhcp_header_sizes[i]);
			q[dhcp_header_sizes[i]] = '\0';
			fr_pair_value_strsteal(vp, q);

			if (vp->vp_length == 0) fr_pair_list_free(&vp);
			break;



How about something like this instead ? (copy a string, but do not trust the DHCP field to be actually null terminated)

		case PW_TYPE_STRING:
			if (*p != '\0') {
				uint8_t *end;
				int len;
				end = memchr(p, '\0', dhcp_header_sizes[i]);
				len = end ? end - p : dhcp_header_sizes[i];
				q = talloc_array(vp, char, len + 1);
				memcpy(q, p, len);
				q[len] = '\0';
				fr_pair_value_strsteal(vp, q);
			}
			if (vp->vp_length == 0) fr_pair_list_free(&vp);
			break;





This would avoid having things like:

        DHCP-Server-Host-Name = "\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000"
        DHCP-Boot-Filename = "\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000"




(I can do a pull request for 3.1.x if you wish)


Regards,
Nicolas.

This message contains information that may be privileged or confidential and is the property of the Capgemini Group. It is intended only for the person to whom it is addressed. If you are not the intended recipient, you are not authorized to read, print, retain, copy, disseminate, distribute, or use this message or any part thereof. If you receive this message in error, please notify the sender immediately and delete all copies of this message.