builtins: make ltrimstr and rtrimstr error for non-string inputs

I also refactored the code in rtrimstr to make it more similar to the code used by ltrimstr. 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 Ref: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=64946
jqlang · Dec 10, 2023 · 98be5a9 · 98be5a9
1 parent 6b69ffe
commit 98be5a9
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 7 deletions.
diff --git a/src/builtin.c b/src/builtin.c
@@ -295,7 +295,13 @@ static jv f_endswith(jq_state *jq, jv a, jv b) {
 }
 
 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 startswith = f_startswith(jq, jv_copy(input), jv_copy(left));
+ if (!jv_is_valid(startswith)) {
+ jv_free(input);
+ jv_free(left);
+ return startswith;
+ }
+ if (jv_get_kind(startswith) != JV_KIND_TRUE) {
  jv_free(left);
  return input;
  }
@@ -311,14 +317,21 @@ static jv f_ltrimstr(jq_state *jq, jv input, jv left) {
 }
 
 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 endswith = f_endswith(jq, jv_copy(input), jv_copy(right));
+ if (!jv_is_valid(endswith)) {
  jv_free(input);
- return res;
+ jv_free(right);
+ return endswith;
  }
- jv_free(right);
- return input;
+ if (jv_get_kind(endswith) != JV_KIND_TRUE) {
+ jv_free(right);
+ return input;
+ }
+ int suffixlen = jv_string_length_bytes(right);
+ jv res = jv_string_sized(jv_string_value(input),
+ jv_string_length_bytes(jv_copy(input)) - suffixlen);
+ jv_free(input);
+ return res;
 }
 
 jv binop_minus(jv a, jv b) {

diff --git a/tests/jq.test b/tests/jq.test
@@ -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 #2967
+
 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"]