"+" string converted to "=2B" in SQL request
Hi everybody, I'am using Freeradius 3.0.12 with backend MySQL. I customized my SQL groupreply request like this: authorize_group_reply_query = "\ SELECT id, groupname, attribute, \ value, op \ FROM ${groupreply_table} \ WHERE groupname = '%{${group_attribute}}' AND value LIKE '%%%{Called-Station-Id}%%' \ ORDER BY id" %(Called-Station-Id) can be phone number like +33567897654, and the request sent to MySQL is: SELECT id, groupname, attribute, value, op FROM radgroupreply WHERE groupname = GROUP' AND value LIKE '%=2B33567897654%' ORDER BY id '+' string was converted to "=2B'. How can I do to preserve + string Best Regards, Tony LEMEUNIER Tony LEMEUNIER
On Mar 5, 2018, at 11:13 AM, Tony LEMEUNIER <Tony.Lemeunier@novelcom.fr> wrote:
I'am using Freeradius 3.0.12 with backend MySQL.
I customized my SQL groupreply request like this:
authorize_group_reply_query = "\ SELECT id, groupname, attribute, \ value, op \ FROM ${groupreply_table} \ WHERE groupname = '%{${group_attribute}}' AND value LIKE '%%%{Called-Station-Id}%%' \ ORDER BY id"
%(Called-Station-Id) can be phone number like +33567897654, and the request sent to MySQL is:
SELECT id, groupname, attribute, value, op FROM radgroupreply WHERE groupname = GROUP' AND value LIKE '%=2B33567897654%' ORDER BY id
'+' string was converted to "=2B'.
Yes. For security. Otherwise, any user could do an SQL injection attack.
How can I do to preserve + string
See raddb/mods-config/sql/main/mysql/queries.conf Uncomment, and edit the "safe_characters" string. And then watch people pwn your database. Because there's no separate list of safe characters for SELECT versus INSERT. We're working on fixing this for v4. i.e. you're better off *not* putting the "+" into the DB. Alan DeKok.
Hi Alan, And thanks for your reply. My radius is on private LAN, so no risk. Best Regards, Tony LEMEUNIER Le 05/03/2018 17:23, « Freeradius-Users au nom de Alan DeKok » <freeradius-users-bounces+tony.lemeunier=novelcom.fr@lists.freeradius.org au nom de aland@deployingradius.com> a écrit : On Mar 5, 2018, at 11:13 AM, Tony LEMEUNIER <Tony.Lemeunier@novelcom.fr> wrote: > I'am using Freeradius 3.0.12 with backend MySQL. > > I customized my SQL groupreply request like this: > > authorize_group_reply_query = "\ > SELECT id, groupname, attribute, \ > value, op \ > FROM ${groupreply_table} \ > WHERE groupname = '%{${group_attribute}}' AND value LIKE '%%%{Called-Station-Id}%%' \ > ORDER BY id" > > %(Called-Station-Id) can be phone number like +33567897654, and the request sent to MySQL is: > > SELECT id, groupname, attribute, value, op FROM radgroupreply WHERE groupname = GROUP' AND value LIKE '%=2B33567897654%' ORDER BY id > > '+' string was converted to "=2B'. Yes. For security. Otherwise, any user could do an SQL injection attack. > How can I do to preserve + string See raddb/mods-config/sql/main/mysql/queries.conf Uncomment, and edit the "safe_characters" string. And then watch people pwn your database. Because there's no separate list of safe characters for SELECT versus INSERT. We're working on fixing this for v4. i.e. you're better off *not* putting the "+" into the DB. Alan DeKok. - List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html
On Mar 5, 2018, at 11:28 AM, Tony LEMEUNIER <Tony.Lemeunier@novelcom.fr> wrote:
And thanks for your reply. My radius is on private LAN, so no risk.
Do you trust all of the users who log in via the RADIUS server? a) yes - you should (mostly) be fine b) no - you will get pwned. It's not about "private LAN". The users are sending *names and passwords* to the RADIUS server, among other data. That data is under the users control, and they can change it to do malicious things. Alan DeKok.
Hi,
And then watch people pwn your database. Because there's no separate list of safe characters for SELECT versus INSERT.
We're working on fixing this for v4.
I've become a big fan of prepared statements to avoid this kind of issue. Is the fix in v4 doing prepared statements? Greetings, Stefan Winter -- Stefan WINTER Ingenieur de Recherche Fondation RESTENA - Réseau Téléinformatique de l'Education Nationale et de la Recherche 2, avenue de l'Université L-4365 Esch-sur-Alzette Tel: +352 424409 1 Fax: +352 422473 PGP key updated to 4096 Bit RSA - I will encrypt all mails if the recipient's key is known to me http://pgp.mit.edu:11371/pks/lookup?op=get&search=0xC0DE6A358A39DC66
On Mar 6, 2018, at 12:52 PM, Stefan Winter <stefan.winter@RESTENA.LU> wrote:
Hi,
And then watch people pwn your database. Because there's no separate list of safe characters for SELECT versus INSERT.
We're working on fixing this for v4.
I've become a big fan of prepared statements to avoid this kind of issue.
Is the fix in v4 doing prepared statements?
There are two ways of doing this, tainted values and prepared statements. For SQL prepared statements are definitely the way to go as there are essentially zero side effects to using them, and they always work predictably. Unfortunately not all datastores allow prepared statements, LDAP being the big one. Where datastores don't support prepared statements the 'taint' state of any boxed values used to construct the query/filter/url are used to determine whether the value should be escaped. Any value created in the config is "untainted" any value from an external source (including databases) is considered "tainted". The major problem with this is where an attribute value could come from the network or locally depending on the contents of the packet being processed. There you need to remember to explicitly mark up your local value as tainted if it contains characters that might need escaping. Alan and I have had long discussions about the advantages and disadvantages of tainted values. I can see having to explicitly taint or escape any dynamic inputs to queries in the default config to get them to work reliably for everyone, at which point the scheme seems to lose some of its value. The one big advantage is that it lets users easily control which values are escaped and which aren't via %{taint:} %{untaint:}, without needing to call explicit escape and unescape functions. Anyway if anyone wants to help out with the prepared statement stuff it is at the stage where its per-driver code that needs writing, i.e. all the infrastructure has been completed, someone just needs to look through and figure out the correct API functions to call. -Arran Arran Cudbard-Bell <a.cudbardb@freeradius.org> FreeRADIUS Development Team FD31 3077 42EC 7FCD 32FE 5EE2 56CF 27F9 30A8 CAA2
On Mar 6, 2018, at 4:37 AM, Arran Cudbard-Bell <a.cudbardb@freeradius.org> wrote:
Alan and I have had long discussions about the advantages and disadvantages of tainted values. I can see having to explicitly taint or escape any dynamic inputs to queries in the default config to get them to work reliably for everyone, at which point the scheme seems to lose some of its value.
The one big advantage is that it lets users easily control which values are escaped and which aren't via %{taint:} %{untaint:}, without needing to call explicit escape and unescape functions.
TBH, it might be good to have something like an "update" section handle this... verify { User-Name =~ /^abcd$/ ... } * for regexes, *all* of the attribute has to match * for '==', it has to match exactly * all other comparisons are disallowed... If the attribute matches, then it's marked as "untainted". That's likely simpler than a generic %{untaint:..} expansion.
Anyway if anyone wants to help out with the prepared statement stuff it is at the stage where its per-driver code that needs writing, i.e. all the infrastructure has been completed, someone just needs to look through and figure out the correct API functions to call.
That would be almost magical in it's usefulness... Alan DeKok.
On Mar 6, 2018, at 7:41 PM, Alan DeKok <aland@deployingradius.com> wrote:
On Mar 6, 2018, at 4:37 AM, Arran Cudbard-Bell <a.cudbardb@freeradius.org> wrote:
Alan and I have had long discussions about the advantages and disadvantages of tainted values. I can see having to explicitly taint or escape any dynamic inputs to queries in the default config to get them to work reliably for everyone, at which point the scheme seems to lose some of its value.
The one big advantage is that it lets users easily control which values are escaped and which aren't via %{taint:} %{untaint:}, without needing to call explicit escape and unescape functions.
TBH, it might be good to have something like an "update" section handle this...
verify { User-Name =~ /^abcd$/ ... }
* for regexes, *all* of the attribute has to match * for '==', it has to match exactly * all other comparisons are disallowed...
If the attribute matches, then it's marked as "untainted". That's likely simpler than a generic %{untaint:..} expansion.
Yeah that'd definitely be useful to do bulk validation of attributes coming in from the network. I was thinking of %{untaint:} being used for something like: update request { User-Name := "%{untaint:%{sql:SELECT * FROM blah WHERE foo}}" } Where the administrator explicitly trusts the result of the query. There's minimal performance penalty because new style xlats are allowed to mutate their inputs and pass them through as outputs, so there's just some reparenting, moving between lists, and setting the taint flag to false. -Arran
participants (4)
-
Alan DeKok -
Arran Cudbard-Bell -
Stefan Winter -
Tony LEMEUNIER