for p1 in mw.ustring.gmatch('apple', '%f[a]') do endinfinite loop
for p1 in mw.ustring.gmatch('apple', 'a*') do endinfinite loop
for p1 in mw.ustring.gmatch('apple', '%f[q]') do endnormal
for p1 in mw.ustring.gmatch('apple', '%f[a]') do endinfinite loop
for p1 in mw.ustring.gmatch('apple', 'a*') do endinfinite loop
for p1 in mw.ustring.gmatch('apple', '%f[q]') do endnormal
| Subject | Author | Repo | Branch | Lines +/- | |
|---|---|---|---|---|---|
| Fix infinite iteration from mw.ustring.gmatch() | Tim Starling | mediawiki/extensions/Scribunto | master | +62 -5 |
The infinite loop isn't good, but I'm uncertain whether this is security worthy just because anyone can trivially write an infinite loop in Lua, which is why we have time limits. I suppose some article/template could be directly passing input to mw.ustring.gmatch, which would allow to trigger the DoS from just article text, but it seems a bit of a stretch to me.
In any case, if this is considered a security issue, I will volunteer to try to fix it.
@Legoktm you can take a look at this if you like, but this is not the highest priority security issue. I'm going to go ahead and untag the security team. Just for clarity, I wanted to post a link to what the security team thinks is the location of the problematic code: https://gerrit.wikimedia.org/g/mediawiki/extensions/Scribunto/+/d67c837b21459118020226931c35dd73702cf887/includes/Engines/LuaCommon/lualib/ustring/ustring.lua#902.
This is publicly documented at https://www.mediawiki.org/wiki/Extension:Scribunto/Lua_reference_manual#mw.ustring.gmatch:
Known bug - When used with a pattern which can match the empty string, the function will get stuck in an infinite loop.
IMO this task could be made public per the comment at T332551#8737062.
An infinite loop in Lua userspace is not a security issue.
The native Lua string.gmatch() increments the match position by 1 if the empty string is matched:
> for w in string.gmatch('1234', '3?') do print('"' .. w .. '"') end
""
""
"3"
""And mw.ustring.gmatch() calls string.gmatch() if the pattern is "simple" so it would seem that we are bound by the same odd convention.
There are version differences.
out = {} for w in string.gmatch('abc', arg[1]) do table.insert(out, '[' .. w .. ']') end print(table.concat(out, ', '))
| Pattern | Lua 5.1 | Lua 5.4 |
|---|---|---|
| <empty> | [], [], [], [] | [], [], [], [] |
| a? | [a], [], [], [] | [a], [], [] |
| b? | [], [b], [], [] | [], [b], [] |
| c? | [], [], [c], [] | [], [], [c] |
| x? | [], [], [], [] | [], [], [], [] |
| %a? | [a], [b], [c], [] | [a], [b], [c] |
| %d? | [], [], [], [] | [], [], [], [] |
The change occurred between 5.2 and 5.3.
I had to compile it with some printf()s to understand what's going on here. In 5.4, you can't have two matches at the same end position. With subject abc and pattern b?, you get a match of "b" with its end position at "c". The zero-length match at "c" is then suppressed because it has the same end position as the previous match of "b". Finally the end-of-string match is returned. So a match with non-zero length suppresses a match of zero length immediately after it.
Thus:
out = {} for w in string.gmatch(arg[1], 'a?') do table.insert(out, '[' .. w .. ']') end print(table.concat(out, ', '))
| Subject | Lua 5.1 | Lua 5.4 |
|---|---|---|
| a1 | [a], [], [] | [a], [] |
| a1a | [a], [], [a], [] | [a], [a] |
| a11a | [a], [], [], [a], [] | [a], [], [a] |
| a11a1 | [a], [], [], [a], [], [] | [a], [], [a], [] |
Change #1321413 had a related patch set uploaded (by Tim Starling; author: Tim Starling):
[mediawiki/extensions/Scribunto@master] Fix infinite iteration from mw.ustring.gmatch()
Change #1321413 merged by jenkins-bot:
[mediawiki/extensions/Scribunto@master] Fix infinite iteration from mw.ustring.gmatch()