Skip to content

Commit b6eec07

Browse files
authored
[protocol] Add spi check to protect stack leaks (#222)
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 <[email protected]>
1 parent c64e2d6 commit b6eec07

File tree

1 file changed

+15
-10
lines changed

1 file changed

+15
-10
lines changed

protocol/spi_proxy.c

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -60,34 +60,39 @@ static void spi_operation_init(struct spi_operation* op) {
6060

6161
static int spi_operation_execute(struct spi_operation* op,
6262
struct libhoth_device* dev) {
63-
uint8_t response_buf[MAX_SPI_OP_PAYLOAD_BYTES];
64-
size_t response_len;
63+
uint8_t response_buf[MAX_SPI_OP_PAYLOAD_BYTES] = {0};
64+
size_t response_len = 0;
6565

66-
// hexdump(op->buf, op->pos);
6766
int status = libhoth_hostcmd_exec(
6867
dev, HOTH_CMD_BOARD_SPECIFIC_BASE + HOTH_PRV_CMD_HOTH_SPI_OPERATION,
6968
/*version=*/0, op->buf, op->pos, response_buf, sizeof(response_buf),
7069
&response_len);
70+
7171
if (status != 0) {
7272
return status;
7373
}
74+
7475
size_t pos = 0;
7576
for (size_t i = 0; i < op->num_transactions; i++) {
7677
struct spi_operation_transaction* transaction = &op->transactions[i];
77-
if (transaction->miso_dest_buf) {
78-
if (transaction->miso_dest_buf_len > 0) {
79-
if (pos + transaction->miso_dest_buf_len > response_len) {
80-
fprintf(stderr,
81-
"returned SPI operation payload is smaller than expected");
82-
return -1;
83-
}
78+
79+
if (transaction->miso_dest_buf_len > 0) {
80+
if (pos + transaction->miso_dest_buf_len + transaction->skip_miso_nbytes >
81+
response_len) {
82+
fprintf(stderr,
83+
"returned SPI operation payload is smaller than expected");
84+
return -1;
85+
}
86+
if (transaction->miso_dest_buf) {
8487
memcpy(transaction->miso_dest_buf,
8588
&response_buf[pos + transaction->skip_miso_nbytes],
8689
transaction->miso_dest_buf_len);
8790
}
8891
}
92+
8993
pos += transaction->skip_miso_nbytes + transaction->miso_dest_buf_len;
9094
}
95+
9196
return 0;
9297
}
9398

0 commit comments

Comments
 (0)