Very rare failure to expand Foreach-Variable

Alan DeKok aland at deployingradius.com
Wed Feb 20 19:10:39 CET 2019


On Feb 20, 2019, at 12:52 PM, Graham Clinch <g.clinch at lancaster.ac.uk> wrote:
> I think:
> 
> - "%{Foreach-Variable-0}" is of type XLAT_VIRTUAL, so it's talloc'd an array of fixed length 2048 (main/xlat.c line 2228)
> - "%{map:...}" is of type XLAT_MODULE, which calls value_data_from_str with src_len=(talloc_array_length(child)-1) = 2047 (main/xlat.c line 2267)

  That's correct for value_data_from_str().  But if the input doesn't contain 2K of data, then value_data_from_str() shouldn't be told it has 2K of data.

> - value_data_from_str loops through every character using the passed src_len of 2047.  After sufficient entropy in the process, occasionally the final character is a backslash, which is bad news and so value_data_from_str returns -1.

  So two issues.  One, the field isn't initialized (oops), and second, the wrong length is being passed to value_data_from_str().

> So I believe this can only occur when correct_escapes is on and an XLAT_MODULE is being called with an XLAT_VIRTUAL child (or an XLAT_MODULE is being called with an XLAT_MODULE child, since XLAT_MODULE also allocs a fixed length array)
> 
> 
> I'm not sure how to move towards a fix here - especially with regard to impact on binary data processing:

  I've pushed a fix. :)

> * xlat_aprint could call value_data_from_str with src_len=strlen() rather than src_len=talloc_array_length() - but then no hope of modules processing binary.
> * xlat_aprint's XLAT_VIRTUAL processing could talloc_realloc to the correct length, after the virtual handler func has returned (do all the funcs return the correct length?)

  Yes, that's it.

> Thanks - this got me straight to the detail.

  Good to hear.

  That's an esoteric bug, and good to have fixed.  Thanks a bunch!

  Alan DeKok.




More information about the Freeradius-Users mailing list