| Submitter | Yuya Nishihara |
|---|---|
| Date | Sept. 5, 2018, 1:58 p.m. |
| Message ID | <33c58a7ff5d54660c5c1.1536155903@mimosa> |
| Download | mbox | patch |
| Permalink | /patch/34336/ |
| State | Accepted |
| Headers | show |
Comments
On Wed, Sep 5, 2018 at 7:00 AM Yuya Nishihara <yuya@tcha.org> wrote: > # HG changeset patch > # User Yuya Nishihara <yuya@tcha.org> > # Date 1536148658 -32400 > # Wed Sep 05 20:57:38 2018 +0900 > # Branch stable > # Node ID 33c58a7ff5d54660c5c15a007d8297119cf9ab82 > # Parent e574cae381b66a8543c8183a76768abcb55d98fc > base85: fix leak on error return from b85decode() > > Spotted by ASAN. > > We don't need to initialize 'out' to NULL, but I decided to do that for > clarity. > Looks like that would lead to a "declaration after statement" warning or whatever it's called. I might drop it in flight if it does indeed cause that. > > diff --git a/mercurial/cext/base85.c b/mercurial/cext/base85.c > --- a/mercurial/cext/base85.c > +++ b/mercurial/cext/base85.c > @@ -77,7 +77,7 @@ static PyObject *b85encode(PyObject *sel > > static PyObject *b85decode(PyObject *self, PyObject *args) > { > - PyObject *out; > + PyObject *out = NULL; > const char *text; > char *dst; > Py_ssize_t len, i, j, olen, cap; > >
On Wed, Sep 5, 2018 at 8:20 AM Martin von Zweigbergk <martinvonz@google.com> wrote: > > > On Wed, Sep 5, 2018 at 7:00 AM Yuya Nishihara <yuya@tcha.org> wrote: > >> # HG changeset patch >> # User Yuya Nishihara <yuya@tcha.org> >> # Date 1536148658 -32400 >> # Wed Sep 05 20:57:38 2018 +0900 >> # Branch stable >> # Node ID 33c58a7ff5d54660c5c15a007d8297119cf9ab82 >> # Parent e574cae381b66a8543c8183a76768abcb55d98fc >> base85: fix leak on error return from b85decode() >> >> Spotted by ASAN. >> >> We don't need to initialize 'out' to NULL, but I decided to do that for >> clarity. >> > > Looks like that would lead to a "declaration after statement" warning or > whatever it's called. I might drop it in flight if it does indeed cause > that. > I was wrong, it's still a declaration even if it contains an initialization. > >> >> diff --git a/mercurial/cext/base85.c b/mercurial/cext/base85.c >> --- a/mercurial/cext/base85.c >> +++ b/mercurial/cext/base85.c >> @@ -77,7 +77,7 @@ static PyObject *b85encode(PyObject *sel >> >> static PyObject *b85decode(PyObject *self, PyObject *args) >> { >> - PyObject *out; >> + PyObject *out = NULL; >> const char *text; >> char *dst; >> Py_ssize_t len, i, j, olen, cap; >> >> >
Patch
diff --git a/mercurial/cext/base85.c b/mercurial/cext/base85.c --- a/mercurial/cext/base85.c +++ b/mercurial/cext/base85.c @@ -77,7 +77,7 @@ static PyObject *b85encode(PyObject *sel static PyObject *b85decode(PyObject *self, PyObject *args) { - PyObject *out; + PyObject *out = NULL; const char *text; char *dst; Py_ssize_t len, i, j, olen, cap; @@ -104,27 +104,33 @@ static PyObject *b85decode(PyObject *sel cap = 4; for (j = 0; j < cap; i++, j++) { c = b85dec[(int)*text++] - 1; - if (c < 0) - return PyErr_Format( + if (c < 0) { + PyErr_Format( PyExc_ValueError, "bad base85 character at position %d", (int)i); + goto bail; + } acc = acc * 85 + c; } if (i++ < len) { c = b85dec[(int)*text++] - 1; - if (c < 0) - return PyErr_Format( + if (c < 0) { + PyErr_Format( PyExc_ValueError, "bad base85 character at position %d", (int)i); + goto bail; + } /* overflow detection: 0xffffffff == "|NsC0", * "|NsC" == 0x03030303 */ - if (acc > 0x03030303 || (acc *= 85) > 0xffffffff - c) - return PyErr_Format( + if (acc > 0x03030303 || (acc *= 85) > 0xffffffff - c) { + PyErr_Format( PyExc_ValueError, "bad base85 sequence at position %d", (int)i); + goto bail; + } acc += c; } @@ -141,6 +147,9 @@ static PyObject *b85decode(PyObject *sel } return out; +bail: + Py_XDECREF(out); + return NULL; } static char base85_doc[] = "Base85 Data Encoding";