Page MenuHomePhabricator

Ex: WikibaseQualityConstraints - remove or sanitize regex for FormatChecker
Closed, ResolvedPublic

Description

As is, the CSV value can be used as a DoS vector, or in the worst case exploit stuff like http://www.openwall.com/lists/oss-security/2015/06/01/6. The regex either needs to be sanitized to a known good expression, or this check needs to be removed

Event Timeline

csteipp raised the priority of this task from to Needs Triage.
csteipp updated the task description. (Show Details)
csteipp added subscribers: Andreasburmeister, csteipp, Tamslo and 4 others.

We read about this and understand it's problematic, but we still have no idea how to fix this issue. For concerns about the runtime we could add a timeout which would lead in the worstcase to a false negative, but about the exploit-stuff-case...

Do you have any advice for us? We were curious how online regex tools handle this problem...

"Sanitizing" usually isn't going to work. The only "solution" is to only allow certain user (usually administrators) to create and maintain these regular expressions.

I'm not sure what kinds of regexes are expected here, so can't give great guidance on the best solution. Theomowmde's solution of only allowing admins to add them will prevent mass exploitation, but would still allow admins to attack the server in the case of another pcre exploit. So I'd prefer to not rely on that.

How important is this feature?

Assuming it's really needed, you could probably do a couple of things,

  • Only allow a subset of regex expressions-- if all you need is for people to say, "\w+" or "[0-9]*", then that should be possible
  • Have a sandboxed service (shell out to a confined binary, or make it a web service) that does the regex processing
  • Implement a descriptive language that always generates safe regexes

We removed the check for the regular expression in the version that should be deployed and are working on a fix for the next version. (https://gerrit.wikimedia.org/r/#/c/218857/ ).
Can we close this ticket?

Jonaskeutel claimed this task.

Please reopen if the removement of the RegEx check does not solve this problem.