Mercurial > hg
changeset 40812:9cdd525d97b2 stable
revlog: fix out-of-bounds access by negative parents read from revlog (SEC)
82d6a35cf432 wasn't enough. Several callers don't check negative revisions
but for -1 (nullrev), which would directly lead to out-of-bounds read, and
buffer overflow could follow. RCE might be doable with carefully crafted
revlog structure, though I don't think this would be useful attack surface.
| author | Yuya Nishihara <yuya@tcha.org> |
|---|---|
| date | Thu, 01 Nov 2018 20:32:59 +0900 |
| parents | 44c2e80db985 |
| children | 884321cd26c3 |
| files | mercurial/cext/revlog.c tests/test-parseindex.t |
| diffstat | 2 files changed, 26 insertions(+), 3 deletions(-) [+] |
line wrap: on
line diff
--- a/mercurial/cext/revlog.c Mon Dec 03 11:14:44 2018 -0800 +++ b/mercurial/cext/revlog.c Thu Nov 01 20:32:59 2018 +0900 @@ -157,6 +157,12 @@ return (const char *)(self->buf.buf) + pos * v1_hdrsize; } +/* + * Get parents of the given rev. + * + * The specified rev must be valid and must not be nullrev. A returned + * parent revision may be nullrev, but is guaranteed to be in valid range. + */ static inline int index_get_parents(indexObject *self, Py_ssize_t rev, int *ps, int maxrev) { @@ -171,7 +177,7 @@ } /* If index file is corrupted, ps[] may point to invalid revisions. So * there is a risk of buffer overflow to trust them unconditionally. */ - if (ps[0] > maxrev || ps[1] > maxrev) { + if (ps[0] < -1 || ps[0] > maxrev || ps[1] < -1 || ps[1] > maxrev) { PyErr_SetString(PyExc_ValueError, "parent out of range"); return -1; }
--- a/tests/test-parseindex.t Mon Dec 03 11:14:44 2018 -0800 +++ b/tests/test-parseindex.t Thu Nov 01 20:32:59 2018 +0900 @@ -133,12 +133,18 @@ $ cd invalidparent $ hg clone --pull -q --config phases.publish=False ../a limit + $ hg clone --pull -q --config phases.publish=False ../a neglimit $ hg clone --pull -q --config phases.publish=False ../a segv - $ rm -R limit/.hg/cache segv/.hg/cache + $ rm -R limit/.hg/cache neglimit/.hg/cache segv/.hg/cache $ "$PYTHON" <<EOF > data = open("limit/.hg/store/00changelog.i", "rb").read() - > for n, p in [(b'limit', b'\0\0\0\x02'), (b'segv', b'\0\x01\0\0')]: + > poisons = [ + > (b'limit', b'\0\0\0\x02'), + > (b'neglimit', b'\xff\xff\xff\xfe'), + > (b'segv', b'\0\x01\0\0'), + > ] + > for n, p in poisons: > # corrupt p1 at rev0 and p2 at rev1 > d = data[:24] + p + data[28:127 + 28] + p + data[127 + 32:] > open(n + b"/.hg/store/00changelog.i", "wb").write(d) @@ -154,6 +160,11 @@ 0 1 1 -1 base 63 62 63 1.01613 63 0 0.00000 1 2 1 -1 base 66 65 66 1.01538 66 0 0.00000 + $ hg -R neglimit debugrevlogindex -f1 -c + rev flag size link p1 p2 nodeid + 0 0000 62 0 -2 -1 7c31755bf9b5 + 1 0000 65 1 0 -2 26333235a41c + $ hg -R segv debugrevlogindex -f1 -c rev flag size link p1 p2 nodeid 0 0000 62 0 65536 -1 7c31755bf9b5 @@ -193,6 +204,12 @@ index_headrevs: parent out of range find_gca_candidates: parent out of range find_deepest: parent out of range + $ "$PYTHON" test.py neglimit/.hg/store + reachableroots: parent out of range + compute_phases_map_sets: parent out of range + index_headrevs: parent out of range + find_gca_candidates: parent out of range + find_deepest: parent out of range $ "$PYTHON" test.py segv/.hg/store reachableroots: parent out of range compute_phases_map_sets: parent out of range