We are using a mysql-backed v3 server and eap / peap / mschapv2. A new user has come along whose email address contains an apostrophe, ('single quote' if you prefer) which is unusual but legitimate. By default we allow users to use their email address as a username. I have found the safe_characters list here: /etc/freeradius/mods-config/sql/main/mysql/queries.conf Without the apostrophe in the safe_characters list, it gets encoded to =27 and so the db query [1] fails to find a user with that username. With the apostrophe in the list, the query fails with "an error in your SQL syntax"... Is there any way out of this conundrum except putting mime-encoded data in the database? [1] authorize_check_query from sql/mysql/dialup.conf
Hi,
We are using a mysql-backed v3 server and eap / peap / mschapv2.
A new user has come along whose email address contains an apostrophe, ('single quote' if you prefer) which is unusual but legitimate.
By default we allow users to use their email address as a username.
Sure. My mail address is stefan';DROP TABLE radacct;@somedomain.com .
I have found the safe_characters list here: /etc/freeradius/mods-config/sql/main/mysql/queries.conf
Without the apostrophe in the safe_characters list, it gets encoded to =27 and so the db query [1] fails to find a user with that username.
If you allow ' as a "safe character" then you open up yourself to SQL injection attacks like the above.
With the apostrophe in the list, the query fails with "an error in your SQL syntax"...
Is there any way out of this conundrum except putting mime-encoded data in the database?
Your query should use %{SQL-User-Name} instead of just %{User-Name}. This contains the escaped version of the username, so they should match. However, as I write this I realise for just how long I haven't worked on that aspect of the server. I might be right though, it's worth a try :-). Greetings, Stefan Winter
[1] authorize_check_query from sql/mysql/dialup.conf - List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html
-- 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 30/10/2018 15:39, Stefan Winter wrote:
Hi,
By default we allow users to use their email address as a username.
Sure. My mail address is stefan';DROP TABLE radacct;@somedomain.com .
Not a problem if the queries are properly escaped or parameterised.
Your query should use %{SQL-User-Name} instead of just %{User-Name}.
It does use %{SQL-User-Name} .
On Oct 30, 2018, at 1:09 PM, Dom Latter <freeradius-users@latter.org> wrote:
On 30/10/2018 15:39, Stefan Winter wrote:
Hi,
By default we allow users to use their email address as a username. Sure. My mail address is stefan';DROP TABLE radacct;@somedomain.com .
Not a problem if the queries are properly escaped or parameterised.
That's what the "safe_characters" configuration does. Allows "safe" characters, and escapes everything else. If you edit the configuration to allow apostrophe, then you *will* be open to attacks, and someone *will* destroy your database. ALan DeKok.
Hi,
Not a problem if the queries are properly escaped or parameterised.
That's what the "safe_characters" configuration does. Allows "safe" characters, and escapes everything else.
Well, to be fair to the OP: using prepared statements would make all those escaping adventures obsolete. In other projects, I learned to love the ability to defer all escaping questions to the library, and just send the stuff I want to send, with peace of mind that this is exactly what will end up in the DB. Greetings, Stefan
If you edit the configuration to allow apostrophe, then you *will* be open to attacks, and someone *will* destroy your database.
ALan DeKok.
- List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html
-- 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
Stefan Winter wrote:
Hi,
Not a problem if the queries are properly escaped or parameterised.
That's what the "safe_characters" configuration does. Allows "safe" characters, and escapes everything else.
Well, to be fair to the OP: using prepared statements would make all those escaping adventures obsolete.
In other projects, I learned to love the ability to defer all escaping questions to the library, and just send the stuff I want to send, with peace of mind that this is exactly what will end up in the DB.
There is an open issue for that: https://github.com/FreeRADIUS/freeradius-server/issues/830 -- Herwin Weststrate
On 30/10/2018 18:06, Stefan Winter wrote:
Hi,
Not a problem if the queries are properly escaped or parameterised.
That's what the "safe_characters" configuration does. Allows "safe" characters, and escapes everything else.
Well, to be fair to the OP: using prepared statements would make all those escaping adventures obsolete.
Or just using conventional escape mechanisms (e.g. mysql_real_escape_string()).
In other projects, I learned to love the ability to defer all escaping questions to the library, and just send the stuff I want to send, with peace of mind that this is exactly what will end up in the DB.
My account management system is written using the Yii PHP framework, and it uses PDO, hence apostrophes etc. safely ending up in the database. It's a long time since I wrote in C but I am guessing that the following added to sql_escape_func() inside rlm_sql.c would sort it: case '\'': if (outlen <= 2) break; out[0] = '\\'; out[1] = '\''; in++; out += 2; outlen -= 2; len += 2; break;
On 02-11-18 11:06, Dom Latter wrote:
On 30/10/2018 18:06, Stefan Winter wrote:
Hi,
Not a problem if the queries are properly escaped or parameterised.
That's what the "safe_characters" configuration does. Allows "safe" characters, and escapes everything else.
Well, to be fair to the OP: using prepared statements would make all those escaping adventures obsolete.
Or just using conventional escape mechanisms (e.g. mysql_real_escape_string()).
The master branch actually has code like that: every SQL backend can have a specialised `sql_escape_func` that uses a sane escape method. It might be a nice thing to backport to v3.0.x as well (with a config option to enable it, it would break backwards compatibility otherwise) -- Herwin Weststrate
On Nov 2, 2018, at 6:06 AM, Dom Latter <freeradius-users@latter.org> wrote:
Or just using conventional escape mechanisms (e.g. mysql_real_escape_string()).
Which, IIRC, wasn't available when the rlm_sql module was written... in 2000 or so. As always, patches are welcome.
My account management system is written using the Yii PHP framework, and it uses PDO, hence apostrophes etc. safely ending up in the database.
When adding them from the account management system. I think there's a misconception here. The issue is *not* about apostrophes in the DB. The issue is apostrophes in SQL queries. And, apostrophes which come from *untrusted user input*. That untrusted user input MUST be escaped for it to be safe. Either that, or passed to a stored procedure. Adding apostrophe to the list of safe characters means that any user can own your database. It is absolutely and 100% the wrong thing to do.
It's a long time since I wrote in C but I am guessing that the following added to sql_escape_func() inside rlm_sql.c would sort it:
That's pretty much what the "safe-characters" code already does. Alan DeKok.
On 02/11/2018 11:04, Alan DeKok wrote:
On Nov 2, 2018, at 6:06 AM, Dom Latter <freeradius-users@latter.org> wrote:
Or just using conventional escape mechanisms (e.g. mysql_real_escape_string()).
Which, IIRC, wasn't available when the rlm_sql module was written... in 2000 or so.
Yes, I suspected that.
I think there's a misconception here. The issue is *not* about apostrophes in the DB. The issue is apostrophes in SQL queries. And, apostrophes which come from *untrusted user input*.
That untrusted user input MUST be escaped for it to be safe. Either that, or passed to a stored procedure.
Adding apostrophe to the list of safe characters means that any user can own your database. It is absolutely and 100% the wrong thing to do.
I am very aware of all this - I should have made myself clearer in the first place. Adding apostrophe to the list was purely an experiment; I had vague hopes that it might have been escaped with a backslash.
It's a long time since I wrote in C but I am guessing that the following added to sql_escape_func() inside rlm_sql.c would sort it:
That's pretty much what the "safe-characters" code already does.
I beg to differ - it mime-encodes. I note that https://dev.mysql.com/doc/refman/5.7/en/mysql-real-escape-string.html says: "Characters encoded are \, ', ", NUL (ASCII 0), \n, \r, and Control+Z. Strictly speaking, MySQL requires only that backslash and the quote character used to quote the string in the query be escaped." So if I have understood, the safe_characters code could be replaced with the snippet I just posted, a similar one for \, and no mime- encoding at all....
On Nov 2, 2018, at 7:32 AM, Dom Latter <freeradius-users@latter.org> wrote:
I am very aware of all this - I should have made myself clearer in the first place. Adding apostrophe to the list was purely an experiment; I had vague hopes that it might have been escaped with a backslash.
The code operates as documented. It doesn't start escaping things *differently* when you turn escaping off...
It's a long time since I wrote in C but I am guessing that the following added to sql_escape_func() inside rlm_sql.c would sort it: That's pretty much what the "safe-characters" code already does.
I beg to differ - it mime-encodes.
It escapes things. The method used is less important.
I note that https://dev.mysql.com/doc/refman/5.7/en/mysql-real-escape-string.html says:
"Characters encoded are \, ', ", NUL (ASCII 0), \n, \r, and Control+Z. Strictly speaking, MySQL requires only that backslash and the quote character used to quote the string in the query be escaped."
So if I have understood, the safe_characters code could be replaced with the snippet I just posted, a similar one for \, and no mime- encoding at all....
It would be *much* preferable to use the mysql_real_escape_string function. That way all knowledge of what to escape is inside of the MySQL code, where it belongs. Alan DeKok.
On 02/11/2018 11:37, Alan DeKok wrote:
On Nov 2, 2018, at 7:32 AM, Dom Latter <freeradius-users@latter.org> wrote:
I beg to differ - it mime-encodes.
It escapes things. The method used is less important.
It's rather important if you are trying to look up a username that contains an apostrophe!
It would be *much* preferable to use the mysql_real_escape_string function. That way all knowledge of what to escape is inside of the MySQL code, where it belongs.
I completely agree. But it's a question of how much budget the customer is prepared to throw at what is essentially a minor problem.
participants (5)
-
Alan DeKok -
Dom Latter -
Herwin Weststrate -
Herwin Weststrate -
Stefan Winter