[MAGNOLIA-3268] SimpleUrlPattern crashes (infinite loop) for certain pattern and URL combinations Created: 04/Aug/10  Updated: 12/Aug/10  Resolved: 09/Aug/10

Status: Closed
Project: Magnolia
Component/s: core
Affects Version/s: 4.3.2
Fix Version/s: 4.3.6

Type: Bug Priority: Critical
Reporter: Benjamin Cody Assignee: Ondrej Chytil
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified
Environment:

1.6.0_07 and 1.6.0_16, win32.


Attachments: Text File SimpleUrlPatternTest.java     Text File magnolia-3268.patch    
Issue Links:
causality
caused by MAGNOLIA-3198 SimpleUrlPattern.URL_CHAR_PATTERN blo... Closed
relation
Template:
Acceptance criteria:
Empty
Task DoD:
[ ]* Doc/release notes changes? Comment present?
[ ]* Downstream builds green?
[ ]* Solution information and context easily available?
[ ]* Tests
[ ]* FixVersion filled and not yet released
[ ]  Architecture Decision Record (ADR)
Bug DoR:
[ ]* Steps to reproduce, expected, and actual results filled
[ ]* Affected version filled
Testcase included:
Yes
Date of First Response:

 Description   

As of 4.3.2, in which the constant URL_CHAR_PATTERN in SimpleUrlPattern was changed, some URL pattern matches cause hang-ups.

import info.magnolia.cms.util.SimpleUrlPattern;

import org.junit.Assert;
import org.junit.Test;

public class SimpleUrlPatternTest
{
   @Test
   public void testDotDoPattern()
   {
      final SimpleUrlPattern sup = new SimpleUrlPattern("*\\.do");
      Assert.assertTrue(sup.match("/path/myController.do")); //This still works
      Assert.assertFalse(sup.match("/.resources/enterprise-css/registration.css")); //This hangs up
   }
}

I noticed this because I have a URL bypass for Spring controller URLs which I've mapped to *.do extensions.

It seems SimpleUrlPattern converts

*\\.do

to

()(.*\\n*)*\\.do

internally, which causes an infinite loop in java.util.regex.Pattern. I tested this with both JDK 1.6.0_07 and 1.6.0_16.



 Comments   
Comment by Jan Haderka [ 04/Aug/10 ]

proposed patch - enable line end matching by . instead of adding
n
to the pattern explicitly.

Comment by Philipp Bärfuss [ 06/Aug/10 ]

I think URL_CHAR_PATTERN is just wrong. It was meant to be a character set and not a pattern and hence should not have any qualifiers as *?+ in it. Even if it doesn't run into an endless loop the performance will be bad.

So yes the patch solves the issue but I would go one step further and deprecate URL_CHAR_PATTERN.

Comment by Fabrizio Giustina [ 06/Aug/10 ]

Just a note about why the "\n" was added: although it's not common to have newlines in a URL without that on a public website any request with a %0A in the URL ends up in displaying the magnolia login page, which is not desired.
Displaying the login page on a public site for a "malformed" request was considered a kind of security threat so we tried to avoid it.

A better solution could be avoiding regexp at all, using something like ant patterns (there should be already a jira somewhere), in the meanwhile we should find a way to fix it without getting back the %0A problem...

Comment by Magnolia International [ 06/Aug/10 ]

Ant-style patterns will also result in 401s because they won't match malformed requests either, and thus the uri-security-filter will get a "false" from the isAuthorized() method; to avoid it completely, maybe we'd a request-validating filter in front ? Something, earlier in the chain, that would return 404s (not sure where it happens at the moment)

Generated at Mon Feb 12 03:44:50 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.