bip32_print_path(): 30-char path splitting for TARGET_BLUE is not correct integrity-wise
https://github.com/LedgerHQ/ledger-app-btc/blob/03d0cbcad04fd84394a8af4de28594deb302b980/src/btchip_helpers.c#L402-L411
Decided to take a quick look at ledger-app-btc code, and noticed that the code referenced above have potential integrity issues, although with current code the issues do not lead to actual problems because bip32_print_path is only ever called on vars.tmp_warning.derivation_pathand vars is a union that contains other member structs that has the size much larger than tmp_warning, and writing beyond derivation_path will just trumple that unused space.
That said, I see two issues that, if not fixed and then bip32_print_path used with other destinations, could lead to data integrity issue that can actually manifest.
-
uint8_t len=strnlen(out, MAX_DERIV_PATH_ASCII_LENGTH);uses constant instead ofmax_out_lenargument, making assumption that is not tested inside the function; I thinkmax_out_lenshould be used instead ofMAX_DERIV_PATH_ASCII_LENGTH -
the loop makes up to 4 moves (MAX_DERIV_PATH_ASCII_LENGTH/30) of the data 'forward', thus extending the length of the data up to 4 bytes, and potentially making the resulting data length 4 bytes longer than the max allowed length, overwriting 4 bytes of unrelated data.
Thanks, a fix will be deployed in the next version of this app :)