From b701aa6c7b698ecd4d4a69d043d2a0c5eab59265 Mon Sep 17 00:00:00 2001 From: Jason Sopko Date: Sat, 5 Sep 2026 23:00:49 -0400 Subject: [PATCH] protocol: Bound the job-validation subcommands to the received length datum_protocol_job_validation_cmd() passed no length to the stxlist, stxlist-by-id and sblock handlers, so they read the DATUM server's frame without knowing how many bytes arrived. stxlist-by-id read a three-byte header and then two bytes per requested id, the id count bounded only against the template's transaction count, never against the received bytes: a short or crafted request reads past the frame and, with a valid job, streams the surplus back to the pool. The reply side of the same exchange was bounded already; this is the request side. Pass the remaining length to each handler, reject a short header, and check that the whole id list arrived before reading it. The test sends a request one byte short of its id list, and one whose count is far past what arrived, each in a buffer of exactly its size; both fail without the check. Found by the new fuzz_protocol_cmd5 harness. --- src/datum_protocol.c | 16 ++++++++++------ src/datum_protocol_tests.c | 26 ++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/src/datum_protocol.c b/src/datum_protocol.c index ff069ba1..17360456 100644 --- a/src/datum_protocol.c +++ b/src/datum_protocol.c @@ -1469,7 +1469,8 @@ int datum_protocol_client_configure(int len, unsigned char *data) { return 1; } -int datum_protocol_job_validation_stxlist(unsigned char *data) { +int datum_protocol_job_validation_stxlist(const int len, const unsigned char * const data) { + if (len < 1) return 0; // similar to compact blocks, we're going to send a list of short transaction IDs for the requested job unsigned char job_index = data[0]; T_DATUM_PROTOCOL_JOB *dj; @@ -1634,11 +1635,13 @@ int datum_protocol_job_validation_stxlist(unsigned char *data) { return 1; } -int datum_protocol_job_validation_stxlist_byid(unsigned char *data) { +int datum_protocol_job_validation_stxlist_byid(const int len, const unsigned char * const data) { + if (len < 3) return 0; // the server is requesting missing transactions // send them unsigned char job_index = data[0]; uint16_t req_count = upk_u16le(data, 1); + if (len < 3 + 2 * req_count) return 0; // the server asked for more ids than it sent T_DATUM_PROTOCOL_JOB *dj; T_DATUM_STRATUM_JOB *sj; @@ -1781,7 +1784,8 @@ int datum_protocol_job_validation_stxlist_byid(unsigned char *data) { return 1; } -int datum_protocol_job_validation_sblock(unsigned char *data) { +int datum_protocol_job_validation_sblock(const int len, const unsigned char * const data) { + if (len < 1) return 0; // the server decided our template probably is too unique from what it knows about, or was // otherwise not able to validate the block using faster negotiations. // It would like us to just send the entire transaction blob for validation as-is. @@ -1961,20 +1965,20 @@ int datum_protocol_job_validation_cmd(int len, unsigned char *data) { switch (cmd) { case 0x10: { // send short txn list - return datum_protocol_job_validation_stxlist(p); + return datum_protocol_job_validation_stxlist(len - 1, p); break; } case 0x11: { // send the requested txns // 16-bit indexes - return datum_protocol_job_validation_stxlist_byid(p); + return datum_protocol_job_validation_stxlist_byid(len - 1, p); break; } case 0x12: { // send the entire block, except the coinbase txn - return datum_protocol_job_validation_sblock(p); + return datum_protocol_job_validation_sblock(len - 1, p); break; } diff --git a/src/datum_protocol_tests.c b/src/datum_protocol_tests.c index babc7eba..657c9a73 100644 --- a/src/datum_protocol_tests.c +++ b/src/datum_protocol_tests.c @@ -828,6 +828,31 @@ static void datum_pow_recycled_protocol_job_test(void) { free(jobs); } +int datum_protocol_job_validation_cmd(int len, unsigned char *data); + +static void datum_protocol_job_validation_bounds_test(void) { + // stxlist-by-id: subcommand 0x11, job index, 16-bit id count, then two + // bytes per id. Each request sits in a buffer of exactly its length, so + // a read past it is a sanitizer report. + unsigned char *req = malloc(1 + 3 + 2 * 3 - 1); // third id one byte short + datum_test(req); + req[0] = 0x11; + req[1] = 0; + pk_u16le(req, 2, 3); + memset(&req[4], 0, 2 * 3 - 1); + datum_test(!datum_protocol_job_validation_cmd(1 + 3 + 2 * 3 - 1, req)); + free(req); + + req = malloc(1 + 3 + 2); // one id sent, 65535 requested + datum_test(req); + req[0] = 0x11; + req[1] = 0; + pk_u16le(req, 2, 0xffff); + memset(&req[4], 0, 2); + datum_test(!datum_protocol_job_validation_cmd(1 + 3 + 2, req)); + free(req); +} + void datum_protocol_tests(void) { datum_protocol_config_v3_tests(); datum_protocol_migration_tests(); @@ -836,4 +861,5 @@ void datum_protocol_tests(void) { datum_protocol_abw_cache_tests(); datum_pow_response_large_difficulty_test(); datum_pow_recycled_protocol_job_test(); + datum_protocol_job_validation_bounds_test(); }