Fix heap buffer overflow in re_parse_char_class #2352
Conversation
There was a problem hiding this comment.
Other than the style issues, the patch looks good.
One comment to the commit message: to get the fixed issues automatically closed when the PR gets merged, all referenced issue numbers must be preceded by a closing keyword as described here: https://help.github.com/articles/closing-issues-using-keywords/#closing-multiple-issues . So, it should read "This patch fixes NNN and fixes MMM."
jerry-core/parser/regexp/re-parser.c
Outdated
| if (is_range == false && lit_utf8_peek_next (parser_ctx_p->input_curr_p) == LIT_CHAR_MINUS) | ||
| if (parser_ctx_p->input_curr_p < parser_ctx_p->input_end_p && | ||
| is_range == false && | ||
| lit_utf8_peek_next (parser_ctx_p->input_curr_p) == LIT_CHAR_MINUS) |
There was a problem hiding this comment.
The formatting of this condition is out of style. It should be
if (cond1
&& cond2
&& cond3)
{
}
jerry-core/parser/regexp/re-parser.c
Outdated
| if (is_range == false && lit_utf8_peek_next (parser_ctx_p->input_curr_p) == LIT_CHAR_MINUS) | ||
| if (parser_ctx_p->input_curr_p < parser_ctx_p->input_end_p && | ||
| is_range == false && | ||
| lit_utf8_peek_next (parser_ctx_p->input_curr_p) == LIT_CHAR_MINUS) |
rerobika
force-pushed
the
issue_2230_and_2237
branch
from
219bed1 to
fcf074b
Compare
|
@akosthekiss Thanks for the review, I've applied your suggestions. |
|
|
||
| try { | ||
| ((new RegExp("[\\u0")).exec("u")); | ||
| } catch (e) { |
There was a problem hiding this comment.
One more note: is it expected that the regex exec throws an exception? In that case we should add an assert (false) after the regex and before the catch IMO. Otherwise the test could pass without checking any assertions.
(same note to all try-catches in both tests)
There was a problem hiding this comment.
Good catch! Also the PR is rebased to the latest master.
rerobika
force-pushed
the
issue_2230_and_2237
branch
from
fcf074b to
8087edd
Compare
This patch fixes jerryscript-project#2230 and fixes jerryscript-project#2237. Test cases are added for both issues and also adds new cases which caused the same error. JerryScript-DCO-1.0-Signed-off-by: Robert Fancsik [email protected]
rerobika
force-pushed
the
issue_2230_and_2237
branch
from
8087edd to
7c3f2bc
Compare
This patch fixes #2230 and fixes #2237.
Test cases are added for both issues and also adds new cases which caused the same error.
JerryScript-DCO-1.0-Signed-off-by: Robert Fancsik [email protected]