Created attachment 15131 [details] Скриншот фрагмента кода В строке 1024 в rpm-4.13.0.1/lib/backend/db3.c происходит сравнение указателя keyp с NULL: if (keyp) {...} Далее в условии в строках 1039-1040 указатель keyp передается на вход функции memcmp(key.data, keyp, keylen), где и происходит разыменование. Имеем следующее: if (keyp) {...} ... for (;;) { ... if (searchType == DBC_PREFIX_SEARCH && (key.size < keylen || memcmp(key.data, keyp, keylen) != 0)) break; ... } Таким образом в memcmp(key.data, keyp, keylen) может произойти разыменование нулевого указателя. Found by Linux Verification Center (https://portal.linuxtesting.ru/) with SVACE. Author A.Burke.
Спасибо за сообщение! Ясно, о чём речь (в функции db3_idxdbGet): раз keyp проверяют в начале, чтобы сделать что-то, только когда оно не NULL, то значит, есть случай, когда оно NULL и будет передано в memcmp в выражении (key.size < keylen || memcmp(key.data, keyp, keylen) != 0). Отметил, что есть случай, когда вызов memcmp с NULL не должен быть страшен (man-страница говорит: If n is zero, the return value is zero). Задумался для начала: когда вообще может вызваться этот memcmp? Необходимо, чтобы был ложен первый дизъюнкт (key.size < keylen). Может ли из этого следовать, что keylen ноль? Ну если мы будем знать, что key.size ноль, то да, будет следовать. (Далее посмотрим код, чтобы понять, какие тут могут быть значения.) Но вообще-то keylen просто должен быть 0 при правильном использовании этой функции! Ведь длина keylen, очевидно, должна соответствовать keyp; при keyp==NULL это бессмыслица и неправильное использование этой функции передать keylen отличное от 0. Это уже одна причина, почему разыменование NULL не произойдёт (с учётом всего написанного). Может ли неправильное использование этой функции (с keylen!=0) быть частью атаки? Я думаю, нет, вряд ли такой вызов возможен в коде, а если и да (что вряд ли), то в итоге просто процесс упадёт. Скорее всего, так задумано, что если keyp==NULL, то вся проверка всегда ложна. Так что было бы яснее дописать в начало if (keyp && ...), чтобы значение keylen никак уже не участвовало в случае keyp==NULL. Или if (keylen > 0 && ...), если мы верим что keylen соответствует keyp. Хотел разобрать внимательнее все возможности значений переменных в этом куске кода (в функции db3_idxdbGet), но ввиду первой причины про keylen==0 это уже не так существенно. Но вот: static rpmRC db3_idxdbGet(dbiIndex dbi, dbiCursor dbc, const char *keyp, size_t keylen, dbiIndexSet *set, int searchType) { rpmRC rc = RPMRC_FAIL; /* assume failure */ if (dbi != NULL && dbc != NULL && set != NULL) { int cflags = DB_NEXT; int dbrc; DBT data, key; memset(&data, 0, sizeof(data)); memset(&key, 0, sizeof(key)); if (keyp) { if (keylen == 0) { /* XXX "/" fixup */ keyp = ""; keylen = 1; } key.data = (void *) keyp; /* discards const */ key.size = keylen; cflags = searchType == DBC_PREFIX_SEARCH ? DB_SET_RANGE : DB_SET; } for (;;) { dbiIndexSet newset = NULL; dbrc = dbiCursorGet(dbc, &key, &data, cflags); if (dbrc != 0) break; if (searchType == DBC_PREFIX_SEARCH && (key.size < keylen || memcmp(key.data, keyp, keylen) != 0)) break; Случай keyp == NULL. Какое key.size в момент интересующей нас проверки (key.size < keylen || memcmp(key.data, keyp, keylen) != 0)? Казалось бы, key.size выставлено в 0 memset-ом. Но неверно предположение, что dbiCursorGet не меняет это значение, как раз наоборот: заполнит найденным ключом. * * * Есть мысль, что этот код, видимо, написан в предположении, что просто эту функцию не вызовут с NULL и DBC_PREFIX_SEARCH, потому что это, видимо, не имеет смысла. (Ведь при keyp==NULL значение searchType никак не влияет на действия -- имею в виду dbiCursorGet. Конечно, может быть ещё вариант, что DBC_PREFIX_SEARCH по задумке в случае keyp==NULL должно влиять на дальнейшие действия в цикле, но вряд ли.) Эта мысль подтверждается тем, что * вызовов idxdbGet с NULL и DBC_PREFIX_SEARCH нет в исходниках (и кстати keylen==0 в таких случаях): $ git -P grep -nEe 'idxdbGet.*NULL' gears/sisyphus gears/sisyphus:lib/rpmdb.c:233: rc = idxdbGet(dbi, dbc, NULL, 0, set, DBC_NORMAL_SEARCH); gears/sisyphus:lib/rpmdb.c:2018: rc = idxdbGet(ii->ii_dbi, ii->ii_dbc, NULL, 0, &ii->ii_set, DBC_NORMAL_SEARCH); $ * вариантом bdbro_idxdbGet из bdb_ro.c в 7cc9eb84a (Add support for reading BDB without the library, 2019-12-19) (= rpm-4.16.0-alpha~86): if (searchType == DBC_PREFIX_SEARCH) { rpmRC rc = RPMRC_NOTFOUND; if (!keyp) return RPMRC_FAIL; Для успокоения можно было бы вставить в код проверку (или даже переписать с разделением на две части для DBC_PREFIX_SEARCH и нет так, как это сделано в bdbro_idxdbGet -- ведь это может быть немного оптимальнее: проверить один раз). Так что это другая причина, почему такого вызова memcmp с NULL не произойдёт. Это опять неправильное использование функции с неправильным сочестанием аргументов. На этот раз соображение про DBC_PREFIX_SEARCH, которые подразумевает не-NULL. * * * Есть ещё соображение, почему обнаруживать передачу NULL в memcmp важно: gcc может считать это undefined behavior и полагать, что там не может быть NULL, и оптимизируя убирать весь код, который выполняется при условии, что там NULL. https://postgrespro.com/list/thread-id/1870065 Но в нашем случае эта гипотетическая опасность не кажется применимой: тут memcmp под двумя условиями: searchType == DBC_PREFIX_SEARCH и на keylen. * * * Вот демонстрационный патч, который можно приложить в следующем релизе rpm, но он добавляет лишнюю проверку (лишнее действие), на то, что не происходит при правильном использовании, поэтому считаю, что применять его не надо (у меня в ветке fix-worries я собираю такие пачти на всякий случай): commit 766d25e5ff3e63e7543e12532c0db21d9103f7dd (@ALT/fix-worries, fix-worries) Author: Ivan Zakharyaschev <imz@altlinux.org> Date: Mon Apr 27 10:52:14 2026 +0300 db3_idxdbGet: extra guard against incorrect use and memcmp(NULL, ...) (ALT#48635) This change represents a way to fix the worry about memcmp(NULL, ...) in: if (searchType == DBC_PREFIX_SEARCH && (key.size < keylen || memcmp(key.data, keyp, keylen) != 0)) break; I deem the added check to be not actually adding anything in real applications, be unnecessary, inappropriate because a correct usage of this function would not lead to such incorrect combinations of arguments that would lead to an actual NULL-dereference in memcmp. (Explanation 1, 2.) Another tricky worry might be that compilers do wrong optimizations if they discover a case with memcmp(NULL, ...): throw away code that works under the condition of keyp==NULL (because this would anyway be undefined behavior). https://postgrespro.com/list/thread-id/1870065 (New C standard "fixes" this and recommends compilers to apply this change also to the older std modes. https://developers.redhat.com/articles/2024/12/11/making-memcpynull-null-0-well-defined ) But in our case, the memcmp call is under the guard of other conditions, so this worry about compilers is not relevant. So this patch is just for demonstration purposes. ## Counter-argument This function already includes checks for the correctness of arguments, so the authors do not consider it to be the caller's responsibility and do not assume that the args are correct here: rpmRC rc = RPMRC_FAIL; /* assume failure */ if (dbi != NULL && dbc != NULL && set != NULL) ## Explanation 1 I assume it's just an incorrect way to use this function (db3_idxdbGet)--to pass searchType==DBC_PREFIX_SEARCH and keyp==NULL; so, actually, this extra check is just not appropriate, because it is the responsibility of the calling code. This view is kinda confirmed by two things: * there are no such calls in actual code: $ git -P grep -nEe 'idxdbGet.*NULL' gears/sisyphus gears/sisyphus:lib/rpmdb.c:233: rc = idxdbGet(dbi, dbc, NULL, 0, set, DBC_NORMAL_SEARCH); gears/sisyphus:lib/rpmdb.c:2018: rc = idxdbGet(ii->ii_dbi, ii->ii_dbc, NULL, 0, &ii->ii_set, DBC_NORMAL_SEARCH); $ git -P grep -nEe 'idxdbGet.*PREFIX' gears/sisyphus gears/sisyphus:lib/rpmdb.c:256: rc = idxdbGet(dbi, dbc, pfx, plen, set, DBC_PREFIX_SEARCH); $ * another variant of idxdbGet actually doing an analogous verification of arguments: bdbro_idxdbGet from bdb_ro.c in 7cc9eb84a (Add support for reading BDB without the library, 2019-12-19) (= rpm-4.16.0-alpha~86): if (searchType == DBC_PREFIX_SEARCH) { rpmRC rc = RPMRC_NOTFOUND; if (!keyp) return RPMRC_FAIL; (Well, yes, I mean, on one side, it does it explicitly, so we might do it, too, but on the other side, nothing reasonable happens, so it's not a set of arguments that has a sense for this kind of function, i.e., this confirms for me that DBC_PREFIX_SEARCH was not thought to be used with NULL.) ## Explanation 2 A correct use would mean for the arguments that keylen corresponds to keyp. I assume that for keyp==NULL, it must be 0. And note that (according to the manpage for memcmp): If n is zero, the return value is zero. So, it most probably does not dereference the pointers, hence no NULL-dereference could happen. The C standard amendment cited above makes it explicitly legal to pass NULL in case of 0 length. ## Rewriting similar to bdbro_idxdbGet BTW, if proceeding with extra rewrites in this function taking care of special DBC_PREFIX_SEARCH cases, we could split it into two cases analogously to bdbro_idxdbGet: if (keyp) { ... } if (searchType != DBC_PREFIX_SEARCH) { if (keyp) cflags = DB_SET; ... } else if (keyp) { cflags = DB_SET_RANGE; ... } // else exit with RPMRC_FAIL Reported in: https://bugzilla.altlinux.org/48635 Reported by: A Burke <aiburke@fobos-nt.ru> Found by: Linux Verification Center (https://portal.linuxtesting.ru/) with SVACE diff --git a/lib/backend/db3.c b/lib/backend/db3.c index 7f302a71a..f6d7ddf55 100644 --- a/lib/backend/db3.c +++ b/lib/backend/db3.c @@ -1014,7 +1014,8 @@ static rpmRC db3_idxdbGet(dbiIndex dbi, dbiCursor dbc, const char *keyp, size_t dbiIndexSet *set, int searchType) { rpmRC rc = RPMRC_FAIL; /* assume failure */ - if (dbi != NULL && dbc != NULL && set != NULL) { + if (dbi != NULL && dbc != NULL && set != NULL && + (searchType != DBC_PREFIX_SEARCH || keyp)) { int cflags = DB_NEXT; int dbrc; DBT data, key;
