radius_xlat chops embedded NULs in cisco-av-pair

[Arran Cudbard-Bell]

On 31 Mar 2014, at 13:23, Kiril <kyrmail at gmail.com> wrote:

>> I'll take a look and see if it's fixed in later versions. It'll almost certainly work fine in 3.0.2.
> 
> I looked into the current git v3 master sources - the main thing
> remais the same in vp_prints_value:

No, it doesn't.

> print.c, line 258
>> /* xlat.c - need to copy raw value verbatim */
>> else if (quote < 0) {
>> return strlcpy(out, vp->vp_strvalue, outlen);
> 
> in other cases fr_print_string is used

If you look at the calls to fr_print_string, you'd see quote < 0 
is only passed in two places, valuepair.c:85 and valuepair.c:114,
both in the radius_compare_vps function, where the output is 
passed to the regular expression comparison functions.

Arguably if were doing comparisons on the raw attribute it should
be a memcpy, especially as regcomp appears to take the length 
of the input string so it can probably deal with embedded NULLs

But, this isn't related to your issue, if you read the concat
code in 3.x.x xlat.c:1911 it's calling vp_aprint, which at 
the moment just does a talloc_strdup, which is as bad as 
strlcpy.

I'm fixing it so it does:

	case PW_TYPE_STRING:
	{
		size_t len;

		/* Gets us the size of the buffer we need to alloc */
		len = fr_print_string_len(vp->vp_strvalue, vp->length);
		p = talloc_array(ctx, char, len + 1); 	/* +1 for '\0' */
		if (!p) return NULL;

		if (fr_print_string(vp->vp_strvalue, vp->length, p, len + 1) != len) {
			talloc_free(p);
			fr_strerror_printf("Incorrect size of buffer allocated to hold escaped string");
			return NULL;
		}
		break;
	}

Where fr_print_string_len is a new function which calculates the
buffer required to hold an escaped version of the string.

After i've fixed that, and written a regression test for 3.x.x I will 
take a look at 2.x.x.

Arran Cudbard-Bell <a.cudbardb at freeradius.org>
FreeRADIUS Development Team

FD31 3077 42EC 7FCD 32FE 5EE2 56CF 27F9 30A8 CAA2

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 881 bytes
Desc: Message signed with OpenPGP using GPGMail
URL: <http://lists.freeradius.org/mailman/private/freeradius-users/attachments/20140331/ac9837d9/attachment.pgp>