From be8be0f0918e62e72d37d79e8ae095bbf7c08710 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 10 Apr 2020 07:49:24 +0200 Subject: [PATCH] shared: fix crash in _NM_UTILS_STRING_TABLE_LOOKUP_DEFINE() If you have a LIST with 7 elements, and you lookup a value that is not in the (sorted) list and would lie before the first element, the binary search will dig down to imin=0, imid=0, imax=0 and strcmp will give positive cmp value (indicating that the searched value is sorted before). Then, we would do "imax = imid - 1;", which wrapped to G_MAXUINT, and the following "if (G_UNLIKELY (imin > imax))" would not hit, resulting in an out of bound access next. The easy fix is to not used unsigned integers. The binary search was adapted from nm_utils_array_find_binary_search() and nm_utils_ptrarray_find_binary_search(), which already used signed integers to avoid this problem. Fixes: 17d9b852c80c ('shared: explicitly implement binary search in NM_UTILS_STRING_TABLE_LOOKUP_DEFINE*()') --- shared/nm-glib-aux/nm-shared-utils.h | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/shared/nm-glib-aux/nm-shared-utils.h b/shared/nm-glib-aux/nm-shared-utils.h index 172eaa7d12..57a26487b8 100644 --- a/shared/nm-glib-aux/nm-shared-utils.h +++ b/shared/nm-glib-aux/nm-shared-utils.h @@ -1736,10 +1736,10 @@ fcn_name (const char *name) \ \ if (G_LIKELY (name)) { \ G_STATIC_ASSERT (G_N_ELEMENTS (LIST) > 1); \ - G_STATIC_ASSERT (G_N_ELEMENTS (LIST) < G_MAXUINT / 2u - 10u); \ - unsigned imin = 0; \ - unsigned imax = (G_N_ELEMENTS (LIST) - 1); \ - unsigned imid = (G_N_ELEMENTS (LIST) - 1) / 2; \ + G_STATIC_ASSERT (G_N_ELEMENTS (LIST) < G_MAXINT / 2 - 10); \ + int imin = 0; \ + int imax = (G_N_ELEMENTS (LIST) - 1); \ + int imid = (G_N_ELEMENTS (LIST) - 1) / 2; \ \ for (;;) { \ const int cmp = strcmp (LIST[imid].name, name); \ @@ -1748,15 +1748,15 @@ fcn_name (const char *name) \ return get_operator (LIST[imid].value); \ \ if (cmp < 0) \ - imin = imid + 1u; \ + imin = imid + 1; \ else \ - imax = imid - 1u; \ + imax = imid - 1; \ \ if (G_UNLIKELY (imin > imax)) \ break; \ \ - /* integer overflow cannot happen, because LIST is shorter than G_MAXUINT/2. */ \ - imid = (imin + imax) / 2u;\ + /* integer overflow cannot happen, because LIST is shorter than G_MAXINT/2. */ \ + imid = (imin + imax) / 2;\ } \ } \ \