search

RfidResearchGroup / proxmark3

Iceman Fork - Proxmark3
http://www.icedev.se
GNU General Public License v3.0
3.72k stars 998 forks source link

Fix `ExchangeAPDUSC()` in `cmdsmartcard.c` #2186

Closed wh201906 closed 8 months ago

wh201906 commented 8 months ago

This fixes #2184 ExchangeAPDUSC() in cmdsmartcard.c doesn't return 1 if the retry fails, which is different from the behavior of the first try. Iso7816ExchangeEx() will treat the negative length as the valid one when the failed retry occurs. This causes overflow when accessing result[*result_len]

github-actions[bot] commented 8 months ago

You are welcome to add an entry to the CHANGELOG.md as well

iceman1001 commented 8 months ago

Great catch!

those return values 0,1 should be PM3_SUCCESS (0) and PM3_E...

wh201906 commented 8 months ago

I just reused the existing code. If you prefer the PM3_SUCCESS and PM3_E... style, I can make a new PR for that.

iceman1001 commented 8 months ago

I know you reused the existing code. We still can make it follow our prefered return values. One of the many places still not converted

Go for it, make a PR with the defined return values

iceman1001 commented 8 months ago

but remember to handle the negative return values when failing..