From 171ba4c0c3e84104998ceeca12a2c03801c693e6 Mon Sep 17 00:00:00 2001 From: Ellis Sarza-Nguyen Date: Mon, 2 Mar 2026 10:56:49 -0800 Subject: [PATCH] [protocol] Add spi check to protect stack leaks In response to a gh issue identifying some memory safety issue, this seeks to fix the bound check of a spi response buffer to prevent potential stack leaks. The spi response buffer adds an additional check to ensure that the total length of the response does not exceed the buffer size. Signed-off-by: Ellis Sarza-Nguyen --- protocol/spi_proxy.c | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/protocol/spi_proxy.c b/protocol/spi_proxy.c index 05a7864..16655b3 100644 --- a/protocol/spi_proxy.c +++ b/protocol/spi_proxy.c @@ -60,34 +60,39 @@ static void spi_operation_init(struct spi_operation* op) { static int spi_operation_execute(struct spi_operation* op, struct libhoth_device* dev) { - uint8_t response_buf[MAX_SPI_OP_PAYLOAD_BYTES]; - size_t response_len; + uint8_t response_buf[MAX_SPI_OP_PAYLOAD_BYTES] = {0}; + size_t response_len = 0; - // hexdump(op->buf, op->pos); int status = libhoth_hostcmd_exec( dev, HOTH_CMD_BOARD_SPECIFIC_BASE + HOTH_PRV_CMD_HOTH_SPI_OPERATION, /*version=*/0, op->buf, op->pos, response_buf, sizeof(response_buf), &response_len); + if (status != 0) { return status; } + size_t pos = 0; for (size_t i = 0; i < op->num_transactions; i++) { struct spi_operation_transaction* transaction = &op->transactions[i]; - if (transaction->miso_dest_buf) { - if (transaction->miso_dest_buf_len > 0) { - if (pos + transaction->miso_dest_buf_len > response_len) { - fprintf(stderr, - "returned SPI operation payload is smaller than expected"); - return -1; - } + + if (transaction->miso_dest_buf_len > 0) { + if (pos + transaction->miso_dest_buf_len + transaction->skip_miso_nbytes > + response_len) { + fprintf(stderr, + "returned SPI operation payload is smaller than expected"); + return -1; + } + if (transaction->miso_dest_buf) { memcpy(transaction->miso_dest_buf, &response_buf[pos + transaction->skip_miso_nbytes], transaction->miso_dest_buf_len); } } + pos += transaction->skip_miso_nbytes + transaction->miso_dest_buf_len; } + return 0; }