This is an archive of the discontinued Mercurial Phabricator instance.

Differential D7030

dirs: fix trivial over-read of input data
ClosedPublic

Authored by durin42 on Oct 9 2019, 11:00 AM.

Details

Reviewers
indygreg
Group Reviewers
hg-reviewers
Commits
rHG2a0774e9d2a8: dirs: fix trivial over-read of input data
Summary

This code, introduced in 8c0a7eeda06d, was intentionally over-reading
an input string to avoid getting a shared string object for a one-byte
input. Unfortunately with an empty input (like in the case of a fuzzer
getting started) this was a trivial over-read and triggered an
AddressSanitizer failure.

I went out of my way to make sure the code still does the
copy-avoidance tricks. I don't think this change will cost us much
performance since the one-character strings should be cached
aggressively anyway.

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

durin42 created this revision.Oct 9 2019, 11:00 AM
indygreg accepted this revision.Oct 9 2019, 11:17 PM
This revision is now accepted and ready to land.Oct 9 2019, 11:17 PM
durin42 added a commit: rHG2a0774e9d2a8: dirs: fix trivial over-read of input data.Oct 9 2019, 11:23 PM
Closed by commit rHG2a0774e9d2a8: dirs: fix trivial over-read of input data (authored by durin42). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/cext/dirs.c (41 lines)

Diff 17031

mercurial/cext/dirs.c

* "protocol" such as mutating immutable objects. But since we only * "protocol" such as mutating immutable objects. But since we only
* mutate objects created in this function or in other well-defined * mutate objects created in this function or in other well-defined
* locations, the references are known so these violations should go * locations, the references are known so these violations should go
* unnoticed. The code for adjusting the length of a PyBytesObject is * unnoticed. The code for adjusting the length of a PyBytesObject is
* essentially a minimal version of _PyBytes_Resize. */ * essentially a minimal version of _PyBytes_Resize. */
while ((pos = _finddir(cpath, pos - 1)) != -1) { while ((pos = _finddir(cpath, pos - 1)) != -1) {
PyObject *val; PyObject *val;
if (pos < 2) {
key = PyBytes_FromStringAndSize(cpath, pos);
if (key == NULL)
goto bail;
} else {
/* It's likely that every prefix already has an entry /* It's likely that every prefix already has an entry
in our dict. Try to avoid allocating and in our dict. Try to avoid allocating and
deallocating a string for each prefix we check. */ deallocating a string for each prefix we check. */
if (key != NULL) if (key != NULL)
((PyBytesObject *)key)->ob_shash = -1; ((PyBytesObject *)key)->ob_shash = -1;
else { else {
/* Force Python to not reuse a small shared string. */ /* We know pos >= 2, so we won't get a small
key = PyBytes_FromStringAndSize(cpath, * shared string. */
pos < 2 ? 2 : pos); key = PyBytes_FromStringAndSize(cpath, pos);
if (key == NULL) if (key == NULL)
goto bail; goto bail;
} }
/* Py_SIZE(o) refers to the ob_size member of the struct. Yes, /* Py_SIZE(o) refers to the ob_size member of
* assigning to what looks like a function seems wrong. */ * the struct. Yes, assigning to what looks
* like a function seems wrong. */
Py_SIZE(key) = pos; Py_SIZE(key) = pos;
((PyBytesObject *)key)->ob_sval[pos] = '\0'; ((PyBytesObject *)key)->ob_sval[pos] = '\0';
}
val = PyDict_GetItem(dirs, key); val = PyDict_GetItem(dirs, key);
if (val != NULL) { if (val != NULL) {
PYLONG_VALUE(val) += 1; PYLONG_VALUE(val) += 1;
if (pos < 2) {
/* This was a short string, so we
* probably got a small shared string
* we can't mutate on the next loop
* iteration. Clear it.
*/
Py_CLEAR(key);
}
break; break;
} }
/* Force Python to not reuse a small shared int. */ /* Force Python to not reuse a small shared int. */
#ifdef IS_PY3K #ifdef IS_PY3K
val = PyLong_FromLong(0x1eadbeef); val = PyLong_FromLong(0x1eadbeef);
#else #else
val = PyInt_FromLong(0x1eadbeef); val = PyInt_FromLong(0x1eadbeef);