radius_xlat chops embedded NULs in cisco-av-pair

[Arran Cudbard-Bell]

On 31 Mar 2014, at 07:00, Kiril <kyrmail at gmail.com> wrote:

> Hi,
> there is a problem under freeradius 2.1.12 when incoming radius packet
> has embedded NUL bytes in the middle of the string attribute
> cisco-av-pair
> in debug packet output it is printed as Cisco-AVPair =
> "release-source=\000\000\000\002"
> but when we use %{Cisco-AVPair[*]} in sql accounting query it is
> expanded as "release-source="
> in previous version 1.x the bahaviour was different - xlat-ed string
> in query was shown as in debug output
> 
> according to RFC 2869 section 5 it is said:
> "Servers and servers and clients MUST be able to deal with embedded
> nulls. RADIUS implementers using C are cautioned not to use strcpy()
> when handling strings."

You might want to fully read the RFC before quoting it.

RFC 2865 states:

      Note that none of the types in RADIUS terminate with a NUL (hex
      00).  In particular, types "text" and "string" in RADIUS do not
      terminate with a NUL (hex 00).  The Attribute has a length field
      and does not use a terminator.  Text contains UTF-8 encoded 10646
      [7] characters and String contains 8-bit binary data.  Servers and
      servers and clients MUST be able to deal with embedded nulls.
      RADIUS implementers using C are cautioned not to use strcpy() when
      handling strings.

      The format of the value field is one of five data types.  Note
      that type "text" is a subset of type "string".

      text      1-253 octets containing UTF-8 encoded 10646 [7]
                characters.  Text of length zero (0) MUST NOT be sent;
                omit the entire attribute instead.

      string    1-253 octets containing binary data (values 0 through
                255 decimal, inclusive).  Strings of length zero (0)
                MUST NOT be sent; omit the entire attribute instead.

In RFC2865 speak a string is binary data, which FreeRADIUS deals with
fine. strcpy and variants will never be used on attributes of type
octets (which is the FR type which maps to 'string').

I'll grant you that "Servers and servers and clients MUST be able to
deal with embedded nulls." is ambiguous as to whether it's referring 
to strings or text or both, but this is later clarified in the same
paragraph with "RADIUS implementers using C are cautioned not to 
use strcpy() when handling strings.

> and radius_xlat function uses strlcpy(out, vp->vp_strvalue, outlen) in
> vp_prints_value, so not the full string vp->vp_strvalue is copied

But that's not the problem. The string (RFC type 'text') should be 
escaped before being added to the concatenation buffer. I'll take
a look and see if it's fixed in later versions. It'll almost certainly
work fine in 3.0.2.

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/c099f662/attachment.pgp>