This is an archive of the discontinued Mercurial Phabricator instance.

Differential D4151

linelog: fix infinite loop vulnerability
ClosedPublic

Authored by quark on Aug 7 2018, 1:24 AM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHG27a54096c92e: linelog: fix infinite loop vulnerability
Summary

Checking len(lines) is not a great way of detecting infinite loops, as
demonstrated in the added test. Therefore check instruction count instead.

The original C implementation does not have this problem. There are a few
other places where the C implementation enforces more strictly, like
a1 <= a2, b1 <= b2, rev > 0. But they are optional.

Test Plan

Add a test. The old code forces the test to time out.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

quark created this revision.Aug 7 2018, 1:24 AM
quark updated this revision to Diff 10032.Aug 7 2018, 1:29 AM
quark edited the summary of this revision. (Show Details)Aug 7 2018, 1:31 AM
Closed by commit rHG27a54096c92e: linelog: fix infinite loop vulnerability (authored by quark). · Explain WhyAug 7 2018, 8:52 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/linelog.py (6 lines)
Mtests/test-linelog.py (9 lines)

Diff 10046

mercurial/linelog.py

# practice. # practice.
self.annotate(max(self._lastannotate.rev, rev)) self.annotate(max(self._lastannotate.rev, rev))
if rev > self._maxrev: if rev > self._maxrev:
self._maxrev = rev self._maxrev = rev
def annotate(self, rev): def annotate(self, rev):
pc = 1 pc = 1
lines = [] lines = []
# Sanity check: if len(lines) is longer than len(program), we executed = 0
# Sanity check: if instructions executed exceeds len(program), we
# hit an infinite loop in the linelog program somehow and we # hit an infinite loop in the linelog program somehow and we
# should stop. # should stop.
while pc is not None and len(lines) < len(self._program): while pc is not None and executed < len(self._program):
inst = self._program[pc] inst = self._program[pc]
lastpc = pc lastpc = pc
pc = inst.execute(rev, pc, lines.append) pc = inst.execute(rev, pc, lines.append)
executed += 1
if pc is not None: if pc is not None:
raise LineLogError( raise LineLogError(
'Probably hit an infinite loop in linelog. Program:\n' + 'Probably hit an infinite loop in linelog. Program:\n' +
self.debugstr()) self.debugstr())
ar = annotateresult(rev, lines, lastpc) ar = annotateresult(rev, lines, lastpc)
self._lastannotate = ar self._lastannotate = ar
return ar return ar

tests/test-linelog.py

ar = ll.annotate(rev) ar = ll.annotate(rev)
self.assertEqual(ll.annotateresult, lines) self.assertEqual(ll.annotateresult, lines)
# Verify we can get back these states by annotating each rev # Verify we can get back these states by annotating each rev
for lines, rev, a1, a2, b1, b2, blines, usevec in _genedits( for lines, rev, a1, a2, b1, b2, blines, usevec in _genedits(
seed, numrevs): seed, numrevs):
ar = ll.annotate(rev) ar = ll.annotate(rev)
self.assertEqual([(l.rev, l.linenum) for l in ar], lines) self.assertEqual([(l.rev, l.linenum) for l in ar], lines)
def testinfinitebadprogram(self):
ll = linelog.linelog.fromdata(
b'\x00\x00\x00\x00\x00\x00\x00\x02' # header
b'\x00\x00\x00\x00\x00\x00\x00\x01' # JUMP to self
)
with self.assertRaises(linelog.LineLogError):
# should not be an infinite loop and raise
ll.annotate(1)
if __name__ == '__main__': if __name__ == '__main__':
import silenttestrunner import silenttestrunner
silenttestrunner.main(__name__) silenttestrunner.main(__name__)