protocol: Bound the job-validation subcommands to the received length

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;
 		}
 

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();
 }