diff --git a/src/lib/testsuite/test_error.c b/src/lib/testsuite/test_error.c index 10a7d54..fd048b3 100644 --- a/src/lib/testsuite/test_error.c +++ b/src/lib/testsuite/test_error.c @@ -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 diff --git a/src/lib/utils.c b/src/lib/utils.c index da042e7..4b18e3a 100644 --- a/src/lib/utils.c +++ b/src/lib/utils.c @@ -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; }