Skip to content

Commit 195258d

Browse files
authored
Fix buffer overflow in set_pin_code() (#802)
set_pin_code() copies the caller-supplied pin string into pin_code with strcpy and no length check, but pin_code is a fixed esp_bt_pin_code_t, i.e. uint8_t[16]. Any pin of 16 characters or more (the Bluetooth spec allows PINs up to 16 digits, and the function takes an arbitrary const char* so nothing stops a caller from passing something longer) overwrites whatever member happens to follow pin_code in memory - pin_code_len right now, but really anything else depending on how the class layout changes over time. This bounds the copy to the size of the buffer and keeps pin_code NUL-terminated. pin_code_len is now set from the actual (possibly truncated) length instead of the untruncated strlen(), so it stays consistent with what actually ended up in pin_code, and the two places that read pin_code/pin_code_len afterwards (the initial esp_bt_gap_set_pin call and the pin_req callback's pin_reply) keep working correctly.
1 parent a39ecc1 commit 195258d

1 file changed

Lines changed: 9 additions & 2 deletions

File tree

src/BluetoothA2DPSource.cpp

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,8 +88,15 @@ void BluetoothA2DPSource::set_pin_code(const char *pin_code,
8888
esp_bt_pin_type_t pin_type) {
8989
ESP_LOGD(BT_APP_TAG, "%s, ", __func__);
9090
this->pin_type = pin_type;
91-
this->pin_code_len = strlen(pin_code);
92-
strcpy((char *)this->pin_code, pin_code);
91+
size_t len = strlen(pin_code);
92+
if (len >= sizeof(this->pin_code)) {
93+
ESP_LOGE(BT_APP_TAG, "pin code is too long - truncating to %d characters",
94+
(int)sizeof(this->pin_code) - 1);
95+
len = sizeof(this->pin_code) - 1;
96+
}
97+
memcpy(this->pin_code, pin_code, len);
98+
this->pin_code[len] = 0;
99+
this->pin_code_len = len;
93100
}
94101

95102
void BluetoothA2DPSource::start(std::vector<const char *> names) {

0 commit comments

Comments
 (0)