Skip to content

Commit a59c6d6

Browse files
committed
Validate opcodes in lre_byte_swap
Fixes: #1376 Check that opcode bytes are < REOP_COUNT before indexing into the reopcode_info array. Also change lre_byte_swap to return int (-1 on error, 0 on success) instead of aborting, and handle the error gracefully.
1 parent fa799e9 commit a59c6d6

4 files changed

Lines changed: 47 additions & 11 deletions

File tree

libregexp.c

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2554,14 +2554,14 @@ const char *lre_get_groupnames(const uint8_t *bc_buf)
25542554
return (const char *)(bc_buf + RE_HEADER_LEN + re_bytecode_len);
25552555
}
25562556

2557-
void lre_byte_swap(uint8_t *buf, size_t len, bool is_byte_swapped)
2557+
int lre_byte_swap(uint8_t *buf, size_t len, bool is_byte_swapped)
25582558
{
25592559
uint8_t *p, *pe;
25602560
uint32_t n, r, nw;
25612561

25622562
p = buf;
25632563
if (len < RE_HEADER_LEN)
2564-
abort();
2564+
return -1;
25652565

25662566
// format is:
25672567
// <header>
@@ -2576,12 +2576,14 @@ void lre_byte_swap(uint8_t *buf, size_t len, bool is_byte_swapped)
25762576
if (is_byte_swapped)
25772577
n = bswap32(n);
25782578
if (n > len - RE_HEADER_LEN)
2579-
abort();
2579+
return -1;
25802580

25812581
p = &buf[RE_HEADER_LEN];
25822582
pe = &p[n];
25832583

25842584
while (p < pe) {
2585+
if (*p >= REOP_COUNT)
2586+
return -1;
25852587
n = reopcode_info[*p].size;
25862588
switch (n) {
25872589
case 1:
@@ -2622,10 +2624,11 @@ void lre_byte_swap(uint8_t *buf, size_t len, bool is_byte_swapped)
26222624
inplace_bswap32(&p[13]);
26232625
break;
26242626
default:
2625-
abort();
2627+
return -1;
26262628
}
26272629
p = &p[n];
26282630
}
2631+
return 0;
26292632
}
26302633

26312634
#ifdef TEST

libregexp.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ int lre_exec(uint8_t **capture,
6060
int lre_parse_escape(const uint8_t **pp, int allow_utf16);
6161
bool lre_is_space(int c);
6262

63-
void lre_byte_swap(uint8_t *buf, size_t len, bool is_byte_swapped);
63+
int lre_byte_swap(uint8_t *buf, size_t len, bool is_byte_swapped);
6464

6565
/* must be provided by the user */
6666
bool lre_check_stack_overflow(void *opaque, size_t alloca_size);

lre-test.c

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,29 @@ static void oob_save_index(void)
4545
assert(ret < 0);
4646
}
4747

48+
// https://github.com/quickjs-ng/quickjs/issues/1376
49+
static void invalid_opcode_byte_swap(void)
50+
{
51+
// Bytecode with an opcode byte >= REOP_COUNT triggers an OOB read
52+
// of the reopcode_info array in lre_byte_swap. Simulate the real
53+
// big-endian deserialization path (is_byte_swapped=true) which is
54+
// how JS_ReadRegExp calls it. The bytecode_len is stored as
55+
// little-endian on disk, so it appears byte-swapped to a BE reader.
56+
uint8_t bc[] = {
57+
0x00, 0x00, // RE_HEADER_FLAGS = 0
58+
0x01, // RE_HEADER_CAPTURE_COUNT = 1
59+
0x00, // RE_HEADER_STACK_SIZE = 0
60+
0x00, 0x00, 0x00, 0x01, // RE_HEADER_BYTECODE_LEN = 1 (big-endian / byte-swapped)
61+
0x1E, // opcode 30 >= REOP_COUNT (invalid)
62+
};
63+
64+
int ret = lre_byte_swap(bc, sizeof(bc), /*is_byte_swapped*/true);
65+
assert(ret < 0);
66+
}
67+
4868
int main(void)
4969
{
5070
oob_save_index();
71+
invalid_opcode_byte_swap();
5172
return 0;
5273
}

quickjs.c

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -37314,13 +37314,20 @@ static int JS_WriteRegExp(BCWriterState *s, JSRegExp regexp)
3731437314

3731537315
JS_WriteString(s, regexp.pattern);
3731637316

37317-
if (is_be())
37318-
lre_byte_swap(str8(bc), bc->len, /*is_byte_swapped*/false);
37317+
if (is_be()) {
37318+
if (lre_byte_swap(str8(bc), bc->len, /*is_byte_swapped*/false)) {
37319+
fail:
37320+
JS_ThrowInternalError(s->ctx, "regex byte swap failed");
37321+
return -1;
37322+
}
37323+
}
3731937324

3732037325
JS_WriteString(s, bc);
3732137326

37322-
if (is_be())
37323-
lre_byte_swap(str8(bc), bc->len, /*is_byte_swapped*/true);
37327+
if (is_be()) {
37328+
if (lre_byte_swap(str8(bc), bc->len, /*is_byte_swapped*/true))
37329+
goto fail;
37330+
}
3732437331

3732537332
return 0;
3732637333
}
@@ -38577,8 +38584,13 @@ static JSValue JS_ReadRegExp(BCReaderState *s)
3857738584
return JS_ThrowInternalError(ctx, "bad regexp bytecode");
3857838585
}
3857938586

38580-
if (is_be())
38581-
lre_byte_swap(str8(bc), bc->len, /*is_byte_swapped*/true);
38587+
if (is_be()) {
38588+
if (lre_byte_swap(str8(bc), bc->len, /*is_byte_swapped*/true)) {
38589+
js_free_string(ctx->rt, pattern);
38590+
js_free_string(ctx->rt, bc);
38591+
return JS_ThrowInternalError(ctx, "bad regexp bytecode");
38592+
}
38593+
}
3858238594

3858338595
return js_regexp_constructor_internal(ctx, JS_UNDEFINED,
3858438596
JS_MKPTR(JS_TAG_STRING, pattern),

0 commit comments

Comments
 (0)