Skip to Main Content

Berkeley DB Family

Announcement

For appeals, questions and feedback, please email oracle-forums_moderators_us@oracle.com

True-Positive (minor) Read-Beyond Array Bug

User_VA11DSep 30 2021

summary
There's a minor read-beyond the length of an array bug in 18.1.40 (found by standard gcc 8 compiler warnings). It's a true-positive read beyond an array, probably either static memory or heap memory. I don't think it's user-facing, though, as it's copying to an array that appears to be a null-terminated c-string.
bug
In the function inside src/repmgr/repmgr_net.c:
static int__repmgr_load_ssl_certificates(env, ssl_ctx)
on line 2546, the function declares:
const char *passwd = "";
while on line 2579 it does:
memcpy(rmgr_ssl_conf->repmgr_key_file_passwd, passwd, sizeof(passwd) + 1);
sizeof(passwd) is incorrect here, as const char *passwd = ""; is a pointer, not a C-style array. As such, sizeof(passwd) usually returns 8 on 64 bit platforms and 4 on 32 bit platforms, meaning this reads either 5 or 9 bytes in total from the string instead of the 2 bytes intended.
possible intention of code
The intention of the code was probably
memcpy(rmgr_ssl_conf->repmgr_key_file_passwd, passwd, strlen(passwd) + 1);
The strlen + 1 could also be replaced with a static 2 instead, if it impacts performance for some reason, but given how it statically knew the length of the string, I would assume gcc/other vendors would also replace the strlen with a constant 1.
probable reason for code
I think what the code is intending to do, though, is set the first character to be a null terminator... which isn't needed as on line 2578 it already memset the storage to 0, which is the same as '\0' assuming normal 8 bit bytes.
Line 2579 therefore most likely is dead code, and the passwd variable can most likely be removed entirely from the function, and the comment updated to reflect that the function dependency is expecting a null-terminated string.
meh
This is more of an annoyance as it's flagging in my gcc build and will probably flag when I go to run a static analyzer on my code later. I don't think it's exploitable, and it shouldn't crash cause it's merely a read rather than a write.
disclaimer
Note: This patch may have unintended consequences and no warranty is given. :D

Comments
Post Details
Added on Sep 30 2021
0 comments
49 views