SQL escaping

Alan DeKok aland at deployingradius.com
Wed Sep 26 12:50:45 CEST 2012


Phil Mayers wrote:
> It should do that - I tested it locally with a pretty big variety of
> xlats on the SQL and LDAP modules, including nested xlats.

  Good, thanks.

> However - TBH I find the code in decode_attribute() and
> rad_copy_variable() a bit hairy.

  The code in decode_attribute() is revolting.  The rad_copy_variable()
should be OK, as it's role is much simpler.

> It wasn't obvious to me for example
> that this:
> 
> %{some %{var}:-other %{text}}
> 
> ...isn't legal syntax. I can think of circumstances that might be useful.

  Yeah.

> If the general approach is ok, I'll apply this patch to our testing
> radius server, and might work up a branch with SQL-native escaping to
> really give it a torture test.

  OK, thanks.

> The main reason I used a malloc there was that I didn't want to truncate
> any output, since all the code upward of the stack uses pointer/length
> arguments. And variable-length arrays:
> 
>  char buf[freespace];
> 
> ...are a GCC-ism. But I take your point, and I guess a static buffer is
> OK - we're reading from a "char buf[8192]" at that point anyway.

  Yeah.  If there's an unused buffer at that point, just re-use it...

> Do you want me to re-do the patch stream or just modify the commit
> if/when it gets merged?

  I don't like pain.  Send a pull request for this work, and another one
for the malloc fix.

  Alan DeKok.


More information about the Freeradius-Devel mailing list