Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(7355)

#18684: Pointers point out of array bound in _sre.c

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 years, 11 months ago by storchaka+cpython
Modified:
4 years, 4 months ago
Reviewers:
pitrou
CC:
haypo, ezio.melotti, mrabarnett, Arfrever, BreamoreBoy, devnull_psf.upfronthosting.co.za, storchaka
Visibility:
Public.

Patch Set 1 #

Total comments: 7

Patch Set 2 #

Patch Set 3 #

Unified diffs Side-by-side diffs Delta from patch set Stats Patch
Misc/NEWS View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
Modules/_sre.c View 1 2 6 chunks +22 lines, -6 lines 0 comments Download
Modules/sre_lib.h View 1 2 5 chunks +20 lines, -9 lines 0 comments Download

Messages

Total messages: 2
AntoinePitrou
http://bugs.python.org/review/18684/diff/8853/Modules/_sre.c File Modules/_sre.c (right): http://bugs.python.org/review/18684/diff/8853/Modules/_sre.c#newcode1375 Modules/_sre.c:1375: if ((ctx->ptr - (char *)state->beginning) / state->charsize < It's ...
5 years, 9 months ago #1
storchaka_gmail.com
5 years, 9 months ago #2
http://bugs.python.org/review/18684/diff/8853/Modules/_sre.c
File Modules/_sre.c (right):

http://bugs.python.org/review/18684/diff/8853/Modules/_sre.c#newcode1375
Modules/_sre.c:1375: if ((ctx->ptr - (char *)state->beginning) / state->charsize
<
On 2013/10/21 19:50:39, AntoinePitrou wrote:
> It's a bit of a pity to do a runtime division here. CPUs usually do this much
> slower than multiplications.

This wouln't be an issue after applying a patch in issue18685. Neither on 2.7.

I could rewrite this condition as

if (ctx->pattern[1] > maxsize[state->charsize] ||
    ctx->ptr - (char *)state->beginning < state->charsize * ctx->pattern[1])

And preset maxsize statically
maxsize[i] = PY_SSIZE_T_MAX / i (i is 1, 2, 4)

http://bugs.python.org/review/18684/diff/8853/Modules/_sre.c#newcode1398
Modules/_sre.c:1398: ctx->pattern += ctx->pattern[0];
On 2013/10/21 19:50:39, AntoinePitrou wrote:
> state->ptr isn't updated anymore in this case. Is it a problem?

It isn't a problem. state->ptr is updated only before jump or return.

http://bugs.python.org/review/18684/diff/8853/Modules/_sre.c#newcode3843
Modules/_sre.c:3843: state->start = NULL;
On 2013/10/21 19:50:39, AntoinePitrou wrote:
> Why doesn't this match the old code? Was there a bug?

The old code uses for start a value > `end` to mark a state after found last
match. And when you call next() on the iterator returned by finditer() after it
exhausted (raised StopIteration), `start` will advance more and more. I use the
NULL value instead. Yes, there are nontrivial changes here.
Sign in to reply to this message.

RSS Feeds Recent Issues | This issue
This is Rietveld 894c83f36cb7+