Add test and fix for buffer over-read in pcre2grep #594
Conversation
cedb976 to
02701bc
Compare
| @@ -99,10 +99,9 @@ POSSIBILITY OF SUCH DAMAGE. | |||
| #include <bzlib.h> | |||
| #endif | |||
|
|
|||
| #include "pcre2_util.h" | |||
|
|
|||
| #define PCRE2_CODE_UNIT_WIDTH 8 | |||
| #include "pcre2.h" | |||
There was a problem hiding this comment.
The original idea of pcre2grep was that it was something an external PCRE2 user could have written. In that spirit, it is not right to #include any of the internal pcre2....h files such as pcre2_util.h or pcre2_internal.h.
There was a problem hiding this comment.
Ah I see. So it's OK for pcre2test to use those, but pcre2grep is an "external" client of the API.
I can do that. It would be useful to have assertions though...
There was a problem hiding this comment.
Yes, that''s exactly it; pcre2test has "internal" knowledge because of what it is and various tests that it can't do without that knowledge. I have to confess that I have never really got to grips with assertions. Entirely my fault! Glad to see this lack is being addressed.
There was a problem hiding this comment.
Note though that the assertions were added to pcre2_util.h instead of other internal header file, thinking it might be useful also for non internal use.
|
I fixed/rewritten the UTF-8 decoding in pcre2grep to be much more cautious about not running over the end of the buffer. The new test now passes in valgrind. |
|
The patch looks good, but I am not a pcre2grep expert. It seems invalid utf is not really supported in pcre2grep, but at least it does not do buffer overruns anymore. As far as I remember pcre2grep is just an "example" tool, not intended for production, although some people use it for whatever reason. |
|
It sounds like you've looked at it @zherczeg - I think I can merge? |
Found by Clang-Analyze in #576
This test fails without the code fix, which required correcting the way the pcre2grep tests are run with valgrind.
The new test passes with the code changes made.