XAP: prevent OOB reads in config blob handler (#18926)

This fixes two bugs:

1. An invalid offset could be specified which wasn't checked to be in
   the bounds of the config blob.
2. The data_len check was incorrect as it would allow reading one byte
   past the config blob lenght.

Before the changes the following operation wouldn't fail:

Assuming we have blob of 64 bytes size and attempt a read with an
offset of 32 and data_len of 32, we actually try to read 32 bytes
starting from the 33. byte in the config blob. This reads exactly
one byte past array. Therefore we have to subtract one byte the get
the correct length.
This commit is contained in:
Stefan Kerkmann 2022-11-01 16:54:56 +01:00 committed by GitHub
parent d84090ec96
commit 1983421b7d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 9 additions and 3 deletions

View File

@ -21,8 +21,12 @@
#include "lighting_map.h"
#include "config_blob_gz.h"
bool get_config_blob_chunk(uint16_t offset, uint8_t *data, uint8_t data_len) {
if (offset + data_len > CONFIG_BLOB_GZ_LEN) {
data_len = CONFIG_BLOB_GZ_LEN - offset;
if (offset >= CONFIG_BLOB_GZ_LEN) {
return false;
}
if (offset + data_len >= CONFIG_BLOB_GZ_LEN) {
data_len = (CONFIG_BLOB_GZ_LEN - 1) - offset;
}
memcpy_P(data, &config_blob_gz[offset], data_len);

View File

@ -71,7 +71,9 @@ bool xap_respond_get_config_blob_chunk(xap_token_t token, const void *data, size
xap_route_qmk_config_blob_chunk_t ret = {0};
bool get_config_blob_chunk(uint16_t offset, uint8_t * data, uint8_t data_len);
get_config_blob_chunk(offset, (uint8_t *)&ret, sizeof(ret));
if (!get_config_blob_chunk(offset, (uint8_t *)&ret, sizeof(ret))) {
return false;
}
return xap_respond_data(token, &ret, sizeof(ret));
}