On 21 Sep 2012, at 15:05, Phil Mayers <p.mayers@IMPERIAL.AC.UK> wrote:
On 21/09/12 13:14, Arran Cudbard-Bell wrote:
I'm presuming that properly escaped strings will be converted back to there unescaped form when they get inserted into the database?
Yes. If the NAS sends:
User-Name = "foo'bar" Some-Attr = "some\ntext"
...then then SQL escape function will generate a string (depending on what the DB driver does for escaping):
insert into x (username,msg) values (E'foo''bar', E'some<newline> text')
...and the value in the SQL column will be:
foo'bar some<newline>text
This probably won't cause that many issues, but should probably be noted somewhere.
Applications that relied on FreeRADIUS to do the escaping of special characters would then be vulnerable to various other kinds of injection attacks.
The idea would be that by using the DB escape code, you eliminate *all* vulnerability to injection attacks. Xlat will always generate correctly quoted strings.
But I think you're talking about other apps that then *read* the data FR has inserted, in which case see below.
I was.
I've copied the safe character code into another xlat function (could you pull and update that as part of your patch set too). In your second patch set please wrap user modifiable attributes e.g. User-Name/User-Password where they're used in INSERT/UPDATE queries with %{encode:}, this should minimise the risk of administrators inadvertently exposing other applications to raw user provided data.
My intention was to make the escaping mode a setting on the SQL instance that defaults to safe-characters, but can be set to "native".
I've not seen any other application that pre-escapes data that it inserts to the database beyond handling the specific requirements of the protocol/markup language. It's not generally expected behaviour, so I don't think it should be the default behaviour. However, the SQL module has been around for nearly a decade now, so it'd be good to provide backwards compatibility with the default config, for attribute that commonly contain user input, hence suggesting the default config still escape User-Name/User-Password. -Arran