Re: Inconsistent escaping of regex parentheses
(First of all, apologies if the formatting of this email formatting isn’t 100% - it’s been a while since I replied to an archived email post on a mailing list and it would appear that using a web based client is a pretty horrific experience out of the box..)
I'm in the process of upgrading an old 2.2.0 installation of FreeRADIUS to a more sane 3.0.17 release (stock from current CentOS).
That's good, but 3.0.20 is available from our web site:
Fair point.. I’ve moved onto the 3.0.20 build while investigating this.
What does the full debug log show?
Using a fresh install of 3.0.20 on CentOS 8, I added the following Unlang procedure to the start of the authorise section within the default server; in order to trigger the behaviour: testRegex { update control { Tmp-String-0 = '@(DOMAIN1|DOMAIN2)$' } if ( &User-Name =~ /%{control:Tmp-String-0}/i ) { update control { Tmp-String-1 = 'Yay!' } } } Results of the regex quoting are technically the same - but here the comparison simply fails rather than returning an error. — v3.0.20 - FreeRADIUS - Official repo (1) policy testRegex { (1) update control { (1) Tmp-String-0 = '@(DOMAIN1|DOMAIN2)$' (1) } # update control = noob (1) if ( &User-Name =~ /%{control:Tmp-String-0}/i ) { (1) EXPAND %{control:Tmp-String-0} (1) --> @\(DOMAIN1\|DOMAIN2)\$ (1) if ( &User-Name =~ /%{control:Tmp-String-0}/i ) -> FALSE (1) } # policy testRegex = noop The different error behaviour made me slightly curious, so I went back to a completely clean v3.0.17 CentOS deployment, used the same Unlang procedure as above - and the originally seen error is seen. — v3.0.17 - CentOS (1) # Executing section authorize from file /etc/raddb/sites-enabled/default (1) authorize { (1) policy testRegex { (1) update control { (1) Tmp-String-0 = '@(DOMAIN1|DOMAIN2)$' (1) } # update control = noop (1) if ( &User-Name =~ /%{control:Tmp-String-0}/i ) { (1) EXPAND %{control:Tmp-String-0} (1) --> @\(DOMAIN1\|DOMAIN2)\$ (1) ERROR: @\(DOMAIN1\|DOMAIN2)\$ (1) ERROR: ^ Pattern compilation failed: unmatched parentheses (1) ERROR: Failed retrieving values required to evaluate condition (1) } # policy testRegex = noop In both cases the regex fails as it is syntactically broken once escaping takes place. Thanks in advance — Tim
On Jan 21, 2020, at 9:37 AM, Tim <tim@yetanother.net> wrote:
Using a fresh install of 3.0.20 on CentOS 8, I added the following Unlang procedure to the start of the authorise section within the default server; in order to trigger the behaviour:
testRegex { update control { Tmp-String-0 = '@(DOMAIN1|DOMAIN2)$' }
if ( &User-Name =~ /%{control:Tmp-String-0}/i ) {
Ah... you're using attributes to define the entire content of a regular expression. That's generally a bad thing to do. The reason is similar to SQL escaping and SQL injection attacks. You *don't* want user input to control what matches and what doesn't.
(1) if ( &User-Name =~ /%{control:Tmp-String-0}/i ) { (1) EXPAND %{control:Tmp-String-0} (1) --> @\(DOMAIN1\|DOMAIN2)\$
The escaping makes sense, mostly. But it should be consistent. i.e. it's not escaping the final ')'.
(1) if ( &User-Name =~ /%{control:Tmp-String-0}/i ) -> FALSE (1) } # policy testRegex = noop
The different error behaviour made me slightly curious, so I went back to a completely clean v3.0.17 CentOS deployment, used the same Unlang procedure as above - and the originally seen error is seen. ...{ (1) EXPAND %{control:Tmp-String-0} (1) --> @\(DOMAIN1\|DOMAIN2)\$ (1) ERROR: @\(DOMAIN1\|DOMAIN2)\$ (1) ERROR: ^ Pattern compilation failed: unmatched parentheses
Hmm... I suspect some other change related to how it uses the regex libraries.
In both cases the regex fails as it is syntactically broken once escaping takes place.
Yes. In short, the expansions inside of regex are for text, not for regex "special characters". This prevents attacks similar to SQL injections. We're working on fixing this in v4, where it will track the "tainted" status of data. Data taken from user input are tainted / untrusted. Data taken from configuration files is untainted, and trusted. So your use-case should work. But in v3, it's just too hard to fix. I'll see if I can figure out why the closing brace isn't escaped. The code is from 2015, and the comments say that this is intentional. I don't recall why. The simple answer is put regular expressions directly into the configuration. Don't put them into attributes. Alan DeKok.
participants (2)
-
Alan DeKok -
Tim