Skip to content

Commit

Permalink
builtins: make ltrimstr and rtrimstr error for non-string inputs
Browse files Browse the repository at this point in the history
Previously, ltrimstr/rtrimstr would just let the input pass through for
non-string inputs or arguments.

That was happening because, they were leaking the errors returned by
startswith/endswith treating them as if they were jv_false().

This patch also fixes that memory leak discovered by oss-fuzz

Instead of fixing it in C, I just reimplemented those builtins in jq.

Ref: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=64946
  • Loading branch information
emanuele6 committed Dec 11, 2023
1 parent 7e54d96 commit decfb30
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 29 deletions.
29 changes: 0 additions & 29 deletions src/builtin.c
Original file line number Diff line number Diff line change
Expand Up @@ -294,33 +294,6 @@ static jv f_endswith(jq_state *jq, jv a, jv b) {
return ret;
}

static jv f_ltrimstr(jq_state *jq, jv input, jv left) {
if (jv_get_kind(f_startswith(jq, jv_copy(input), jv_copy(left))) != JV_KIND_TRUE) {
jv_free(left);
return input;
}
/*
* FIXME It'd be better to share the suffix with the original input --
* that we could do, we just can't share prefixes.
*/
int prefixlen = jv_string_length_bytes(left);
jv res = jv_string_sized(jv_string_value(input) + prefixlen,
jv_string_length_bytes(jv_copy(input)) - prefixlen);
jv_free(input);
return res;
}

static jv f_rtrimstr(jq_state *jq, jv input, jv right) {
if (jv_get_kind(f_endswith(jq, jv_copy(input), jv_copy(right))) == JV_KIND_TRUE) {
jv res = jv_string_sized(jv_string_value(input),
jv_string_length_bytes(jv_copy(input)) - jv_string_length_bytes(right));
jv_free(input);
return res;
}
jv_free(right);
return input;
}

jv binop_minus(jv a, jv b) {
if (jv_get_kind(a) == JV_KIND_NUMBER && jv_get_kind(b) == JV_KIND_NUMBER) {
jv r = jv_number(jv_number_value(a) - jv_number_value(b));
Expand Down Expand Up @@ -1732,8 +1705,6 @@ BINOPS
{f_keys_unsorted, "keys_unsorted", 1},
{f_startswith, "startswith", 2},
{f_endswith, "endswith", 2},
{f_ltrimstr, "ltrimstr", 2},
{f_rtrimstr, "rtrimstr", 2},
{f_string_split, "split", 2},
{f_string_explode, "explode", 1},
{f_string_implode, "implode", 1},
Expand Down
2 changes: 2 additions & 0 deletions src/builtin.jq
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ def fromdateiso8601: strptime("%Y-%m-%dT%H:%M:%SZ")|mktime;
def todateiso8601: strftime("%Y-%m-%dT%H:%M:%SZ");
def fromdate: fromdateiso8601;
def todate: todateiso8601;
def ltrimstr($left): if startswith($left) then .[$left | length:] end;
def rtrimstr($right): if endswith($right) then .[:$right | -length] end;
def match(re; mode): _match_impl(re; mode; false)|.[];
def match($val): ($val|type) as $vt | if $vt == "string" then match($val; null)
elif $vt == "array" and ($val | length) > 1 then match($val[0]; $val[1])
Expand Down
20 changes: 20 additions & 0 deletions tests/jq.test
Original file line number Diff line number Diff line change
Expand Up @@ -2091,7 +2091,27 @@ try ("foobar" | .[1.5]) catch .
null
"Cannot index string with number"


# setpath/2 does not leak the input after an invalid get #2970

try ["ok", setpath([1]; 1)] catch ["ko", .]
{"hi":"hello"}
["ko","Cannot index object with number"]


# ltrimstr/1 and rtrimstr/1 don't leak, and return an error for
# non-string arguments. #2969

.[] as [$x, $y] | try ["ok", ($x | ltrimstr($y))] catch ["ko", .]
[["hi",1],[1,"hi"],["hi","hi"],[1,1]]
["ko","startswith() requires string inputs"]
["ko","startswith() requires string inputs"]
["ok",""]
["ko","startswith() requires string inputs"]

.[] as [$x, $y] | try ["ok", ($x | rtrimstr($y))] catch ["ko", .]
[["hi",1],[1,"hi"],["hi","hi"],[1,1]]
["ko","endswith() requires string inputs"]
["ko","endswith() requires string inputs"]
["ok",""]
["ko","endswith() requires string inputs"]

0 comments on commit decfb30

Please sign in to comment.