Skip to content

Commit fa799e9

Browse files
committed
Validate save index in lre_exec_backtrack
Fixes: #1375 Replace assert() with runtime check for REOP_save_start/REOP_save_end/ REOP_save_reset capture indices. Also add lre-test with a test for this issue, and the starting point for more lre tests.
1 parent eb2ba4f commit fa799e9

7 files changed

Lines changed: 123 additions & 8 deletions

File tree

.github/workflows/ci.yml

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,10 @@ jobs:
169169
run: |
170170
./build/api-test
171171
172+
- name: test lre
173+
run: |
174+
./build/lre-test
175+
172176
windows-msvc:
173177
runs-on: ${{ matrix.config.os }}
174178
strategy:
@@ -205,6 +209,9 @@ jobs:
205209
- name: test api
206210
run: |
207211
build\${{matrix.config.buildType}}\api-test.exe
212+
- name: test lre
213+
run: |
214+
build\${{matrix.config.buildType}}\lre-test.exe
208215
- name: Set up Visual Studio shell
209216
uses: egor-tensin/vs-shell@v2
210217
with:
@@ -245,6 +252,9 @@ jobs:
245252
- name: test api
246253
run: |
247254
build\${{matrix.buildType}}\api-test.exe
255+
- name: test lre
256+
run: |
257+
build\${{matrix.buildType}}\lre-test.exe
248258
249259
windows-ninja:
250260
runs-on: windows-latest
@@ -277,6 +287,9 @@ jobs:
277287
- name: test api
278288
run: |
279289
build\api-test.exe
290+
- name: test lre
291+
run: |
292+
build\lre-test.exe
280293
281294
windows-sdk:
282295
runs-on: windows-latest
@@ -310,6 +323,9 @@ jobs:
310323
- name: test api
311324
run: |
312325
build\${{matrix.buildType}}\api-test.exe
326+
- name: test lre
327+
run: |
328+
build\${{matrix.buildType}}\lre-test.exe
313329
314330
windows-mingw:
315331
runs-on: windows-latest
@@ -364,6 +380,9 @@ jobs:
364380
- name: test api
365381
run: |
366382
./build/api-test
383+
- name: test lre
384+
run: |
385+
./build/lre-test
367386
windows-mingw-shared:
368387
runs-on: windows-latest
369388
defaults:
@@ -457,6 +476,10 @@ jobs:
457476
run: |
458477
./build/api-test
459478
479+
- name: test lre
480+
run: |
481+
./build/lre-test
482+
460483
openbsd:
461484
runs-on: ubuntu-latest
462485
steps:

CMakeLists.txt

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -435,6 +435,13 @@ add_executable(api-test
435435
target_compile_definitions(api-test PRIVATE ${qjs_defines})
436436
target_link_libraries(api-test PRIVATE qjs)
437437

438+
add_executable(lre-test
439+
lre-test.c
440+
libregexp.c
441+
libunicode.c
442+
)
443+
target_compile_definitions(lre-test PRIVATE ${qjs_defines})
444+
438445
# Unicode generator
439446
#
440447

libregexp.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2250,7 +2250,8 @@ static intptr_t lre_exec_backtrack(REExecContext *s, uint8_t **capture,
22502250
case REOP_save_start:
22512251
case REOP_save_end:
22522252
val = *pc++;
2253-
assert(val < s->capture_count);
2253+
if (val >= s->capture_count)
2254+
return LRE_RET_BYTECODE_ERROR;
22542255
capture[2 * val + opcode - REOP_save_start] = (uint8_t *)cptr;
22552256
break;
22562257
case REOP_save_reset:
@@ -2259,7 +2260,8 @@ static intptr_t lre_exec_backtrack(REExecContext *s, uint8_t **capture,
22592260
val = pc[0];
22602261
val2 = pc[1];
22612262
pc += 2;
2262-
assert(val2 < s->capture_count);
2263+
if (val2 >= s->capture_count)
2264+
return LRE_RET_BYTECODE_ERROR;
22632265
while (val <= val2) {
22642266
capture[2 * val] = NULL;
22652267
capture[2 * val + 1] = NULL;

libregexp.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,9 @@ extern "C" {
4343
#define LRE_FLAG_NAMED_GROUPS (1 << 7) /* named groups are present in the regexp */
4444
#define LRE_FLAG_UNICODE_SETS (1 << 8)
4545

46-
#define LRE_RET_MEMORY_ERROR (-1)
47-
#define LRE_RET_TIMEOUT (-2)
46+
#define LRE_RET_MEMORY_ERROR (-1)
47+
#define LRE_RET_TIMEOUT (-2)
48+
#define LRE_RET_BYTECODE_ERROR (-3)
4849

4950
uint8_t *lre_compile(int *plen, char *error_msg, int error_msg_size,
5051
const char *buf, size_t buf_len, int re_flags,

lre-test.c

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
#ifdef NDEBUG
2+
#undef NDEBUG
3+
#endif
4+
#include <assert.h>
5+
#include <stdlib.h>
6+
#include <string.h>
7+
#include "libregexp.h"
8+
9+
bool lre_check_stack_overflow(void *opaque, size_t alloca_size)
10+
{
11+
return false;
12+
}
13+
14+
int lre_check_timeout(void *opaque)
15+
{
16+
return 0;
17+
}
18+
19+
void *lre_realloc(void *opaque, void *ptr, size_t size)
20+
{
21+
if (size == 0) {
22+
free(ptr);
23+
return NULL;
24+
}
25+
return realloc(ptr, size);
26+
}
27+
28+
// https://github.com/quickjs-ng/quickjs/issues/1375
29+
static void oob_save_index(void)
30+
{
31+
// Bytecode with REOP_save_start index=100, but capture_count=1.
32+
// Without validation this causes a heap-buffer-overflow in lre_exec_backtrack.
33+
uint8_t bc[] = {
34+
0x00, 0x00, // RE_HEADER_FLAGS = 0
35+
0x01, // RE_HEADER_CAPTURE_COUNT = 1
36+
0x00, // RE_HEADER_STACK_SIZE = 0
37+
0x04, 0x00, 0x00, 0x00, // RE_HEADER_BYTECODE_LEN = 4 (little-endian)
38+
0x05, // REOP_any
39+
0x0C, 0x64, // REOP_save_start, index=100
40+
0x0B, // REOP_match
41+
};
42+
43+
uint8_t *capture[2] = {NULL, NULL};
44+
int ret = lre_exec(capture, bc, (const uint8_t *)"a", 0, 1, 0, NULL);
45+
assert(ret < 0);
46+
}
47+
48+
int main(void)
49+
{
50+
oob_save_index();
51+
return 0;
52+
}

meson.build

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -521,6 +521,20 @@ if tests.allowed()
521521
),
522522
)
523523

524+
# LRE test
525+
test(
526+
'lre',
527+
executable(
528+
'lre-test',
529+
'lre-test.c',
530+
'libregexp.c',
531+
'libunicode.c',
532+
533+
c_args: qjs_c_args,
534+
build_by_default: false,
535+
),
536+
)
537+
524538
# Function.toString() test
525539
test(
526540
'function_source',

quickjs.c

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47536,10 +47536,18 @@ static JSValue js_regexp_exec(JSContext *ctx, JSValueConst this_val,
4753647536
goto fail;
4753747537
}
4753847538
} else {
47539-
if (rc == LRE_RET_TIMEOUT) {
47539+
switch(rc) {
47540+
case LRE_RET_TIMEOUT:
4754047541
JS_ThrowInterrupted(ctx);
47541-
} else {
47542+
break;
47543+
case LRE_RET_MEMORY_ERROR:
4754247544
JS_ThrowInternalError(ctx, "out of memory in regexp execution");
47545+
break;
47546+
case LRE_RET_BYTECODE_ERROR:
47547+
JS_ThrowInternalError(ctx, "corrupted bytecode in regexp execution");
47548+
break;
47549+
default:
47550+
abort();
4754347551
}
4754447552
goto fail;
4754547553
}
@@ -47726,10 +47734,18 @@ static JSValue JS_RegExpDelete(JSContext *ctx, JSValueConst this_val, JSValue ar
4772647734
goto fail;
4772747735
}
4772847736
} else {
47729-
if (ret == LRE_RET_TIMEOUT) {
47737+
switch(ret) {
47738+
case LRE_RET_TIMEOUT:
4773047739
JS_ThrowInterrupted(ctx);
47731-
} else {
47740+
break;
47741+
case LRE_RET_MEMORY_ERROR:
4773247742
JS_ThrowInternalError(ctx, "out of memory in regexp execution");
47743+
break;
47744+
case LRE_RET_BYTECODE_ERROR:
47745+
JS_ThrowInternalError(ctx, "corrupted bytecode in regexp execution");
47746+
break;
47747+
default:
47748+
abort();
4773347749
}
4773447750
goto fail;
4773547751
}

0 commit comments

Comments
 (0)