SQL escaping

Alan DeKok aland at deployingradius.com
Tue Sep 25 21:16:53 CEST 2012


Phil Mayers wrote:
> The last few commits actually make use of the new argument to
> radius_xlat, specifically the SQL modules "safe-characters" is now
> per-instance, and not a static global variable. Which is good.

  That's good.

> So, there are no changes to SQL escaping method - just the addition and
> basic use of escape function context/request arguments.
> 
> If you think this is ok and "pull" it, I'll work up patches next week to
> actually add driver-based SQL escaping as an option. I agree we should
> leave the default as-is.

  I'll try to review the code.

  The only thing for me is that the escaping has to be done recursively.
   e.g. SQL asking for an expansion of "foo %{Bar}" means that the "foo"
portion should be copied as-is.  But the %{Bar} portion is untrusted,
and MUST be escaped.

  From looking at the code, it looks pretty good.  My only $0.02 is:

https://github.com/philmayers/freeradius-server/commit/08de5a57f202aebabdbd0a50a99574aca79a130d

  It does a malloc.  I'd just declare (or re-use) a buffer on the stack.
 malloc() is slow and painful for temporary storage.

  Alan DeKok.


More information about the Freeradius-Devel mailing list