From 4730a6982f2ec70c6e24cbfceff817cf59979074 Mon Sep 17 00:00:00 2001 From: Rucha Deodhar Date: Tue, 19 Apr 2022 21:43:31 +0530 Subject: [PATCH] MDEV-28350: Test failing on buildbot with UBSAN Analysis: There were two kinds of failing tests on buildbot with UBSAN. 1) runtime error: signed integer overflow and 2) runtime error: load of value is not valid value for type Signed integer overflow was occuring because addition of two integers (size of json array + item number in array) was causing overflow in json_path_parts_compare. This overflow happens because a->n_item_end wasn't set. The second error was occuring because c_path->p.types_used is not initialized but the value is used later on to check for negative path index. Fix: For signed integer overflow, use a->n_item_end only in case of range so that it is set. --- sql/item_jsonfunc.cc | 68 ++++++++++++++++++++++++++------------------ strings/json_lib.c | 25 +++++++++------- 2 files changed, 54 insertions(+), 39 deletions(-) diff --git a/sql/item_jsonfunc.cc b/sql/item_jsonfunc.cc index d2d4fc087c9..acf5c199a02 100644 --- a/sql/item_jsonfunc.cc +++ b/sql/item_jsonfunc.cc @@ -844,18 +844,21 @@ String *Item_func_json_extract::read_json(String *str, for (n_arg=1; n_arg < arg_count; n_arg++) { json_path_with_flags *c_path= paths + n_arg - 1; + c_path->p.types_used= JSON_PATH_KEY_NULL; if (!c_path->parsed) { String *s_p= args[n_arg]->val_str(tmp_paths + (n_arg-1)); - if (s_p && - json_path_setup(&c_path->p,s_p->charset(),(const uchar *) s_p->ptr(), - (const uchar *) s_p->ptr() + s_p->length())) + if (s_p) { - report_path_error(s_p, &c_path->p, n_arg); - goto return_null; + if (json_path_setup(&c_path->p,s_p->charset(),(const uchar *) s_p->ptr(), + (const uchar *) s_p->ptr() + s_p->length())) + { + report_path_error(s_p, &c_path->p, n_arg); + goto return_null; + } + c_path->parsed= c_path->constant; + has_negative_path|= c_path->p.types_used & JSON_PATH_NEGATIVE_INDEX; } - c_path->parsed= c_path->constant; - has_negative_path|= c_path->p.types_used & JSON_PATH_NEGATIVE_INDEX; } if (args[n_arg]->null_value) @@ -1391,15 +1394,18 @@ longlong Item_func_json_contains_path::val_int() json_path_with_flags *c_path= paths + n_arg - 2; if (!c_path->parsed) { - String *s_p= args[n_arg]->val_str(tmp_paths+(n_arg-2)); - if (s_p && - json_path_setup(&c_path->p,s_p->charset(),(const uchar *) s_p->ptr(), - (const uchar *) s_p->ptr() + s_p->length())) + String *s_p= args[n_arg]->val_str(tmp_paths + (n_arg-2)); + if (s_p) { - report_path_error(s_p, &c_path->p, n_arg-2); - goto return_null; + if (json_path_setup(&c_path->p,s_p->charset(),(const uchar *) s_p->ptr(), + (const uchar *) s_p->ptr() + s_p->length())) + { + report_path_error(s_p, &c_path->p, n_arg); + goto null_return; + } + c_path->parsed= c_path->constant; + has_negative_path|= c_path->p.types_used & JSON_PATH_NEGATIVE_INDEX; } - c_path->parsed= c_path->constant; } if (args[n_arg]->null_value) @@ -1460,18 +1466,21 @@ longlong Item_func_json_contains_path::val_int() for (n_arg=2; n_arg < arg_count; n_arg++) { json_path_with_flags *c_path= paths + n_arg - 2; + c_path->p.types_used= JSON_PATH_KEY_NULL; if (!c_path->parsed) { String *s_p= args[n_arg]->val_str(tmp_paths + (n_arg-2)); - if (s_p && - json_path_setup(&c_path->p,s_p->charset(),(const uchar *) s_p->ptr(), - (const uchar *) s_p->ptr() + s_p->length())) + if (s_p) { - report_path_error(s_p, &c_path->p, n_arg); - goto null_return; + if (json_path_setup(&c_path->p,s_p->charset(),(const uchar *) s_p->ptr(), + (const uchar *) s_p->ptr() + s_p->length())) + { + report_path_error(s_p, &c_path->p, n_arg); + goto null_return; + } + c_path->parsed= c_path->constant; + has_negative_path|= c_path->p.types_used & JSON_PATH_NEGATIVE_INDEX; } - c_path->parsed= c_path->constant; - has_negative_path|= c_path->p.types_used & JSON_PATH_NEGATIVE_INDEX; } if (args[n_arg]->null_value) goto null_return; @@ -3633,18 +3642,21 @@ String *Item_func_json_search::val_str(String *str) for (n_arg=4; n_arg < arg_count; n_arg++) { json_path_with_flags *c_path= paths + n_arg - 4; + c_path->p.types_used= JSON_PATH_KEY_NULL; if (!c_path->parsed) { String *s_p= args[n_arg]->val_str(tmp_paths + (n_arg-4)); - if (s_p && - json_path_setup(&c_path->p,s_p->charset(),(const uchar *) s_p->ptr(), - (const uchar *) s_p->ptr() + s_p->length())) + if (s_p) { - report_path_error(s_p, &c_path->p, n_arg); - goto null_return; + if (json_path_setup(&c_path->p,s_p->charset(),(const uchar *) s_p->ptr(), + (const uchar *) s_p->ptr() + s_p->length())) + { + report_path_error(s_p, &c_path->p, n_arg); + goto null_return; + } + c_path->parsed= c_path->constant; + has_negative_path|= c_path->p.types_used & JSON_PATH_NEGATIVE_INDEX; } - c_path->parsed= c_path->constant; - has_negative_path|= c_path->p.types_used & JSON_PATH_NEGATIVE_INDEX; } if (args[n_arg]->null_value) goto null_return; diff --git a/strings/json_lib.c b/strings/json_lib.c index 2df6593d573..e9f671c5a97 100644 --- a/strings/json_lib.c +++ b/strings/json_lib.c @@ -1373,6 +1373,8 @@ static int handle_match(json_engine_t *je, json_path_t *p, (int) (next_step->type & JSON_PATH_KEY_OR_ARRAY)) return json_skip_level(je); + array_counters[next_step - p->steps]= 0; + if (next_step->type & JSON_PATH_ARRAY) { int array_size; @@ -1891,21 +1893,22 @@ int json_path_parts_compare( { if (b->type & JSON_PATH_ARRAY) { - int res= 0, corrected_n_item_a= 0, corrected_n_item_end_a= 0; + int res= 0, corrected_n_item_a= 0; if (array_sizes) - { - corrected_n_item_a= a->n_item < 0 ? array_sizes[b-temp_b] + - a->n_item : - a->n_item; - corrected_n_item_end_a= a->n_item_end < 0 ? array_sizes[b-temp_b] + - a->n_item_end : - a->n_item_end; - } + corrected_n_item_a= a->n_item < 0 ? + array_sizes[b-temp_b] + a->n_item : a->n_item; if (a->type & JSON_PATH_ARRAY_RANGE) + { + int corrected_n_item_end_a= 0; + if (array_sizes) + corrected_n_item_end_a= a->n_item_end < 0 ? + array_sizes[b-temp_b] + a->n_item_end : + a->n_item_end; res= b->n_item >= corrected_n_item_a && - b->n_item <= corrected_n_item_end_a; + b->n_item <= corrected_n_item_end_a; + } else - res= corrected_n_item_a == b->n_item; + res= corrected_n_item_a == b->n_item; if ((a->type & JSON_PATH_WILD) || res) goto step_fits;