wdgzyjx/myxce
1
0
Fork 0
forked from lei.chen/myxce
Code Pull requests Activity

fix(libxce): xce_strerror_r global-buffer-overflow on HA error codes (utils.c)

Browse source 
The E_XCE_* enum (errnox.h) gained NOMASTER/REDIRECT/NOT_READY/STANDBY
(10011-10014) but xce_err_str[] still had only 10 strings + NULL, while the
bounds check used E_XCE_MAX. Formatting any of those codes ran off the array:
NOMASTER -> strncpy(NULL) deref; NOT_READY (10013, returned by ctrlstatus on a
failover-exhausted cluster) -> read 8 bytes past the 88-byte table = the ASAN
global-buffer-overflow at utils.c:335.

Fix: fill the table for all 14 codes; bound xce_strerror_r against the actual
array size + NULL (degrade to 'unknown error', never OOB); add a _Static_assert
tying the table length to E_XCE_MAX so any future enum drift fails the build.
Pre-existing foundation bug surfaced (not caused) by controller-status 6.4.

RED: test_strerror_all_xce_codes SEGV'd at utils.c:335 under ASAN; GREEN: lib
suite Ok:1 Fail:0. New tests in test_error.c cover all in-range + out-of-range codes.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Rongsong Shen 2026-06-20 10:13:21 +00:00
parent 9a0fec667b
commit 1aa0761c12
2 changed files with 80 additions and 13 deletions
Download patch file Download diff file Expand all files Collapse all files

43
src/lib/testsuite/test_error.c
View file

@ -35,11 +35,54 @@ static void test_strerror_r(void) {
}
}
#endif
/* Every XCE error code in [XCE_ERRNO_BASE, E_XCE_MAX) must map to a real string
* via xce_strerror_r — no NULL deref, no out-of-bounds read of xce_err_str[].
* Regression for the global-buffer-overflow at utils.c:335: the HA codes
* E_XCE_NOMASTER/REDIRECT/NOT_READY/STANDBY (10011-10014) were added to the
* enum but not to xce_err_str[], so formatting them (e.g. ctrlstatus on a
* failover-exhausted cluster returning E_XCE_NOT_READY) ran off the array.
* Under ASAN this crashes pre-fix. */
static void test_strerror_all_xce_codes(void)
{
char buf[128];
int code;
for (code = XCE_ERRNO_BASE; code < E_XCE_MAX; code++) {
buf[0] = '\0';
int rc = xce_strerror_r(code, buf, sizeof(buf));
TEST_ASSERT_EQUAL_INT(0, rc); /* every in-range code resolves */
TEST_ASSERT_TRUE(buf[0] != '\0'); /* to a non-empty string */
TEST_ASSERT_TRUE(strcmp(buf, "unknown error") != 0); /* a SPECIFIC one */
}
/* the specific HA codes that triggered the crash */
xce_strerror_r(E_XCE_NOT_READY, buf, sizeof(buf));
TEST_ASSERT_TRUE(buf[0] != '\0' && strcmp(buf, "unknown error") != 0);
xce_strerror_r(E_XCE_NOMASTER, buf, sizeof(buf));
TEST_ASSERT_TRUE(buf[0] != '\0' && strcmp(buf, "unknown error") != 0);
}
/* Out-of-range XCE codes (>= E_XCE_MAX) must fall back to "unknown error",
* never index past xce_err_str[] (defense in depth against future enum drift). */
static void test_strerror_out_of_range(void)
{
char buf[128];
TEST_ASSERT_EQUAL_INT(-1, xce_strerror_r(E_XCE_MAX, buf, sizeof(buf)));
TEST_ASSERT_TRUE(strcmp(buf, "unknown error") == 0);
TEST_ASSERT_EQUAL_INT(-1, xce_strerror_r(E_XCE_MAX + 50, buf, sizeof(buf)));
TEST_ASSERT_TRUE(strcmp(buf, "unknown error") == 0);
}
void test_error(void)
{
printf("\n>> error test\n\n");
RUN_TEST(test_error_setget);
RUN_TEST(test_strerror_all_xce_codes);
RUN_TEST(test_strerror_out_of_range);
#if 0
RUN_TEST(test_strerror_r);
#endif

50
src/lib/utils.c
View file

@ -309,30 +309,54 @@ void set_error(char* file, char* func, int line, int code)
ectx_t* get_ectx() { return &ectx; }
static char* xce_err_str[] = { "invalid job",
"invalid worker",
"duplicated jobs",
"duplicated workers",
"bad opcode",
"unknown role",
"unknown resource",
"unknown queue",
"unknown group",
"internal error",
NULL };
/* One string per E_XCE_* code, in enum order (errnox.h). Index = code -
* XCE_ERRNO_BASE. MUST stay in lockstep with the enum — the _Static_assert
* below fails the build if a code is added to errnox.h without a string here
* (that drift caused a global-buffer-overflow in xce_strerror_r: bug-NOT_READY,
* the HA codes 10011-10014 had no entries). */
static char* xce_err_str[] = { "invalid job", /* E_XCE_BAD_JOB */
"invalid worker", /* E_XCE_BAD_WORKER */
"duplicated jobs", /* E_XCE_DUP_JOB */
"duplicated workers", /* E_XCE_DUP_WORKER */
"bad opcode", /* E_XCE_BAD_OPCODE */
"unknown role", /* E_XCE_BAD_ROLE */
"unknown resource", /* E_XCE_BAD_RES */
"unknown queue", /* E_XCE_BAD_QUEUE */
"unknown group", /* E_XCE_BAD_GROUP */
"internal error", /* E_XCE_INTERNAL */
"no known primary", /* E_XCE_NOMASTER */
"redirect to primary", /* E_XCE_REDIRECT */
"controller not ready", /* E_XCE_NOT_READY */
"controller is standby" };/* E_XCE_STANDBY */
_Static_assert(sizeof(xce_err_str) / sizeof(xce_err_str[0])
== (size_t)(E_XCE_MAX - XCE_ERRNO_BASE),
"xce_err_str[] is out of sync with the E_XCE_* enum (errnox.h): "
"add a string for every new error code");
int xce_strerror_r(int errnum, char* buf, size_t bufsize)
{
if (bufsize == 0) {
return -1;
}
if (errnum < XCE_ERRNO_BASE) {
return strerror_r(errnum, buf, bufsize);
}
if (errnum >= E_XCE_MAX) {
/* Defense in depth: bound against the ACTUAL table size (not just E_XCE_MAX)
* and reject NULL slots, so a future enum/table drift degrades to "unknown
* error" instead of reading past xce_err_str[]. */
size_t idx = (size_t)(errnum - XCE_ERRNO_BASE);
size_t nstr = sizeof(xce_err_str) / sizeof(xce_err_str[0]);
if (errnum >= E_XCE_MAX || idx >= nstr || xce_err_str[idx] == NULL) {
strncpy(buf, "unknown error", bufsize);
buf[bufsize - 1] = '\0';
return -1;
}
strncpy(buf, xce_err_str[errnum - XCE_ERRNO_BASE], bufsize);
strncpy(buf, xce_err_str[idx], bufsize);
buf[bufsize - 1] = '\0';
return 0;
}