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/08de5a57f202aebabdbd0... 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.