[PATCH] Input/mouse: cyapa - fix stack overflow bug in cyapa_gen3_get_query_data

[PATCH] Input/mouse: cyapa - fix stack overflow bug in cyapa_gen3_get_query_data

[itewqq]

The i2c_smbus_read_block_data function receives up to I2C_SMBUS_BLOCK_MAX
bytes. which is typically 32. This exceeds the size of the local variable
(u8 query_data[QUERY_DATA_SIZE]) in cyapa_gen3_get_query_data, which is
provided to cyapa_read_block and finally reach i2c_smbus_read_block_data.
When the cyapa module is enabled (CONFIG_MOUSE_CYAPA=m), this bug could
cause denial-of-service (or potentially code execution). For example, by a
physical attacker who can hijack I2C communications or plant malicious
firmware in the Cyapa peripheral. To fix this bug, this patch change the
size of query_data from QUERY_DATA_SIZE to I2C_SMBUS_BLOCK_MAX.

Signed-off-by: itewqq <shipeiqu@sjtu.edu.cn>
---
 drivers/input/mouse/cyapa_gen3.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/mouse/cyapa_gen3.c b/drivers/input/mouse/cyapa_gen3.c
index fc3fb954523b..6a5ffff51922 100644
--- a/drivers/input/mouse/cyapa_gen3.c
+++ b/drivers/input/mouse/cyapa_gen3.c
@@ -980,7 +980,7 @@ static int cyapa_gen3_set_proximity(struct cyapa *cyapa, bool enable)
 
 static int cyapa_gen3_get_query_data(struct cyapa *cyapa)
 {
-	u8 query_data[QUERY_DATA_SIZE];
+	u8 query_data[I2C_SMBUS_BLOCK_MAX];
 	int ret;
 
 	if (cyapa->state != CYAPA_STATE_OP)
-- 
2.30.2

[PATCH v2] Input/mouse: cyapa - fix potential buffer overflow in cyapa_gen3.c

[itewqq]

The i2c_smbus_read_block_data function receives up to I2C_SMBUS_BLOCK_MAX
bytes, which is defined as 32. This exceeds the size of the struct
cyapa_reg_data, which will be provided to cyapa_read_block as an input
buffer and finally reach i2c_smbus_read_block_data. When the cyapa module
is enabled (CONFIG_MOUSE_CYAPA=m), this bug could result in potential
denial-of-service for invalid or malicious I2C data. Pad the size of the
cyapa_reg_data structure from 27 to I2C_SMBUS_BLOCK_MAX=32 bytes to
address this issue.

Signed-off-by: itewqq <shipeiqu@sjtu.edu.cn>
---
 drivers/input/mouse/cyapa_gen3.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/input/mouse/cyapa_gen3.c b/drivers/input/mouse/cyapa_gen3.c
index fc3fb954523b..6297d1376bbe 100644
--- a/drivers/input/mouse/cyapa_gen3.c
+++ b/drivers/input/mouse/cyapa_gen3.c
@@ -31,7 +31,7 @@
 
 /* Macro for register map group offset. */
 #define PRODUCT_ID_SIZE  16
-#define QUERY_DATA_SIZE  27
+#define QUERY_DATA_SIZE  32
 #define REG_PROTOCOL_GEN_QUERY_OFFSET  20
 
 #define REG_OFFSET_DATA_BASE     0x0000
@@ -114,6 +114,8 @@ struct cyapa_reg_data {
 	u8 finger_btn;
 	/* CYAPA reports up to 5 touches per packet. */
 	struct cyapa_touch touches[5];
+	/* padding up to size I2C_SMBUS_BLOCK_MAX. */
+	u8 padding[5];
 } __packed;
 
 struct gen3_write_block_cmd {
-- 
2.30.2

Re: [PATCH v2] Input/mouse: cyapa - fix potential buffer overflow in cyapa_gen3.c

[Dmitry Torokhov]

Hi,

On Thu, Oct 17, 2024 at 04:01:04PM +0800, itewqq wrote:
> The i2c_smbus_read_block_data function receives up to I2C_SMBUS_BLOCK_MAX
> bytes, which is defined as 32. This exceeds the size of the struct
> cyapa_reg_data, which will be provided to cyapa_read_block as an input
> buffer and finally reach i2c_smbus_read_block_data. When the cyapa module
> is enabled (CONFIG_MOUSE_CYAPA=m), this bug could result in potential
> denial-of-service for invalid or malicious I2C data. Pad the size of the
> cyapa_reg_data structure from 27 to I2C_SMBUS_BLOCK_MAX=32 bytes to
> address this issue.

No, I don't think padding all buffers is a good idea. We need to change
i2c_smbus_read_block_data() to accept the buffer size so that it does
not copy more than it should.

I sent a patch to i2c list and CCed you.

Thanks.

-- 
Dmitry