Skip to content

Commit de21386

Browse files
committed
Fix signed integer overflow in jvp_array_write and jvp_object_rehash
This commit fixes signed integer overflow and SEGV issues on growing arrays and objects. The size of arrays and objects is now limited to `536870912` (`0x20000000`). This fixes CVE-2024-23337 and fixes #3262.
1 parent 4e2ccc4 commit de21386

3 files changed

Lines changed: 45 additions & 13 deletions

File tree

src/jv.c

Lines changed: 36 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1025,6 +1025,11 @@ jv jv_array_set(jv j, int idx, jv val) {
10251025
jv_free(val);
10261026
return jv_invalid_with_msg(jv_string("Out of bounds negative array index"));
10271027
}
1028+
if (idx > (INT_MAX >> 2) - jvp_array_offset(j)) {
1029+
jv_free(j);
1030+
jv_free(val);
1031+
return jv_invalid_with_msg(jv_string("Array index too large"));
1032+
}
10281033
// copy/free of val,j coalesced
10291034
jv* slot = jvp_array_write(&j, idx);
10301035
jv_free(*slot);
@@ -1044,6 +1049,7 @@ jv jv_array_concat(jv a, jv b) {
10441049
// FIXME: could be faster
10451050
jv_array_foreach(b, i, elem) {
10461051
a = jv_array_append(a, elem);
1052+
if (!jv_is_valid(a)) break;
10471053
}
10481054
jv_free(b);
10491055
return a;
@@ -1321,6 +1327,7 @@ jv jv_string_indexes(jv j, jv k) {
13211327
}
13221328

13231329
a = jv_array_append(a, jv_number(n));
1330+
if (!jv_is_valid(a)) break;
13241331
p++;
13251332
}
13261333
}
@@ -1369,14 +1376,17 @@ jv jv_string_split(jv j, jv sep) {
13691376

13701377
if (seplen == 0) {
13711378
int c;
1372-
while ((jstr = jvp_utf8_next(jstr, jend, &c)))
1379+
while ((jstr = jvp_utf8_next(jstr, jend, &c))) {
13731380
a = jv_array_append(a, jv_string_append_codepoint(jv_string(""), c));
1381+
if (!jv_is_valid(a)) break;
1382+
}
13741383
} else {
13751384
for (p = jstr; p < jend; p = s + seplen) {
13761385
s = _jq_memmem(p, jend - p, sepstr, seplen);
13771386
if (s == NULL)
13781387
s = jend;
13791388
a = jv_array_append(a, jv_string_sized(p, s - p));
1389+
if (!jv_is_valid(a)) break;
13801390
// Add an empty string to denote that j ends on a sep
13811391
if (s + seplen == jend && seplen != 0)
13821392
a = jv_array_append(a, jv_string(""));
@@ -1394,8 +1404,10 @@ jv jv_string_explode(jv j) {
13941404
const char* end = i + len;
13951405
jv a = jv_array_sized(len);
13961406
int c;
1397-
while ((i = jvp_utf8_next(i, end, &c)))
1407+
while ((i = jvp_utf8_next(i, end, &c))) {
13981408
a = jv_array_append(a, jv_number(c));
1409+
if (!jv_is_valid(a)) break;
1410+
}
13991411
jv_free(j);
14001412
return a;
14011413
}
@@ -1669,10 +1681,13 @@ static void jvp_object_free(jv o) {
16691681
}
16701682
}
16711683

1672-
static jv jvp_object_rehash(jv object) {
1684+
static int jvp_object_rehash(jv *objectp) {
1685+
jv object = *objectp;
16731686
assert(JVP_HAS_KIND(object, JV_KIND_OBJECT));
16741687
assert(jvp_refcnt_unshared(object.u.ptr));
16751688
int size = jvp_object_size(object);
1689+
if (size > INT_MAX >> 2)
1690+
return 0;
16761691
jv new_object = jvp_object_new(size * 2);
16771692
for (int i=0; i<size; i++) {
16781693
struct object_slot* slot = jvp_object_get_slot(object, i);
@@ -1685,7 +1700,8 @@ static jv jvp_object_rehash(jv object) {
16851700
}
16861701
// references are transported, just drop the old table
16871702
jv_mem_free(jvp_object_ptr(object));
1688-
return new_object;
1703+
*objectp = new_object;
1704+
return 1;
16891705
}
16901706

16911707
static jv jvp_object_unshare(jv object) {
@@ -1714,27 +1730,32 @@ static jv jvp_object_unshare(jv object) {
17141730
return new_object;
17151731
}
17161732

1717-
static jv* jvp_object_write(jv* object, jv key) {
1733+
static int jvp_object_write(jv* object, jv key, jv **valpp) {
17181734
*object = jvp_object_unshare(*object);
17191735
int* bucket = jvp_object_find_bucket(*object, key);
17201736
struct object_slot* slot = jvp_object_find_slot(*object, key, bucket);
17211737
if (slot) {
17221738
// already has the key
17231739
jvp_string_free(key);
1724-
return &slot->value;
1740+
*valpp = &slot->value;
1741+
return 1;
17251742
}
17261743
slot = jvp_object_add_slot(*object, key, bucket);
17271744
if (slot) {
17281745
slot->value = jv_invalid();
17291746
} else {
1730-
*object = jvp_object_rehash(*object);
1747+
if (!jvp_object_rehash(object)) {
1748+
*valpp = NULL;
1749+
return 0;
1750+
}
17311751
bucket = jvp_object_find_bucket(*object, key);
17321752
assert(!jvp_object_find_slot(*object, key, bucket));
17331753
slot = jvp_object_add_slot(*object, key, bucket);
17341754
assert(slot);
17351755
slot->value = jv_invalid();
17361756
}
1737-
return &slot->value;
1757+
*valpp = &slot->value;
1758+
return 1;
17381759
}
17391760

17401761
static int jvp_object_delete(jv* object, jv key) {
@@ -1834,7 +1855,11 @@ jv jv_object_set(jv object, jv key, jv value) {
18341855
assert(JVP_HAS_KIND(object, JV_KIND_OBJECT));
18351856
assert(JVP_HAS_KIND(key, JV_KIND_STRING));
18361857
// copy/free of object, key, value coalesced
1837-
jv* slot = jvp_object_write(&object, key);
1858+
jv* slot;
1859+
if (!jvp_object_write(&object, key, &slot)) {
1860+
jv_free(object);
1861+
return jv_invalid_with_msg(jv_string("Object too big"));
1862+
}
18381863
jv_free(*slot);
18391864
*slot = value;
18401865
return object;
@@ -1859,6 +1884,7 @@ jv jv_object_merge(jv a, jv b) {
18591884
assert(JVP_HAS_KIND(a, JV_KIND_OBJECT));
18601885
jv_object_foreach(b, k, v) {
18611886
a = jv_object_set(a, k, v);
1887+
if (!jv_is_valid(a)) break;
18621888
}
18631889
jv_free(b);
18641890
return a;
@@ -1878,6 +1904,7 @@ jv jv_object_merge_recursive(jv a, jv b) {
18781904
jv_free(elem);
18791905
a = jv_object_set(a, k, v);
18801906
}
1907+
if (!jv_is_valid(a)) break;
18811908
}
18821909
jv_free(b);
18831910
return a;

src/jv_aux.c

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -194,18 +194,19 @@ jv jv_set(jv t, jv k, jv v) {
194194
if (slice_len < insert_len) {
195195
// array is growing
196196
int shift = insert_len - slice_len;
197-
for (int i = array_len - 1; i >= end; i--) {
197+
for (int i = array_len - 1; i >= end && jv_is_valid(t); i--) {
198198
t = jv_array_set(t, i + shift, jv_array_get(jv_copy(t), i));
199199
}
200200
} else if (slice_len > insert_len) {
201201
// array is shrinking
202202
int shift = slice_len - insert_len;
203-
for (int i = end; i < array_len; i++) {
203+
for (int i = end; i < array_len && jv_is_valid(t); i++) {
204204
t = jv_array_set(t, i - shift, jv_array_get(jv_copy(t), i));
205205
}
206-
t = jv_array_slice(t, 0, array_len - shift);
206+
if (jv_is_valid(t))
207+
t = jv_array_slice(t, 0, array_len - shift);
207208
}
208-
for (int i=0; i < insert_len; i++) {
209+
for (int i = 0; i < insert_len && jv_is_valid(t); i++) {
209210
t = jv_array_set(t, start + i, jv_array_get(jv_copy(v), i));
210211
}
211212
jv_free(v);

tests/jq.test

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,10 @@ null
226226
[0,1,2]
227227
[0,5,2]
228228

229+
try (.[999999999] = 0) catch .
230+
null
231+
"Array index too large"
232+
229233
#
230234
# Multiple outputs, iteration
231235
#

0 commit comments

Comments
 (0)