Skip to content

Commit

Permalink
src: use uint16_t for length of APDU buffer
Browse files Browse the repository at this point in the history 
When we call ledger_apdu_cache_flush() we may end up combining some
leftover data from the cache with a full 255-byte packet from the
APDU buffer. We must use uint16 for the length counter or it will
roll over.
  • Loading branch information
@pinheadmz
pinheadmz committed May 5, 2021
1 parent 5a60024 commit a4ac01e
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 28 deletions.
4 changes: 2 additions & 2 deletions src/apdu-pubkey.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,11 +121,11 @@ encode_xpub(
return b58enc(b58, b58_sz, data, sizeof(data));
}

uint8_t
uint16_t
hns_apdu_get_public_key(
uint8_t p1,
uint8_t p2,
uint8_t len,
uint16_t len,
volatile uint8_t *buf,
volatile uint8_t *out,
volatile uint8_t *flags
Expand Down
21 changes: 11 additions & 10 deletions src/apdu-signature.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ static ledger_blake2b_ctx blake2;
static inline bool
parse_item(
volatile uint8_t **buf,
uint8_t *len,
uint16_t *len,
uint8_t *item,
size_t item_sz,
ledger_blake2b_ctx *hash
Expand Down Expand Up @@ -193,7 +193,7 @@ parse_item(
static inline bool
parse_addr(
volatile uint8_t **buf,
uint8_t *len,
uint16_t *len,
uint8_t *addr_hash,
uint8_t *addr_len,
ledger_blake2b_ctx *hash
Expand Down Expand Up @@ -228,7 +228,7 @@ parse_addr(
static inline bool
parse_name(
volatile uint8_t **buf,
uint8_t *len,
uint16_t *len,
char *name,
uint8_t *name_len,
ledger_blake2b_ctx *hash
Expand Down Expand Up @@ -269,7 +269,7 @@ parse_name(
static inline bool
cmp_name(
volatile uint8_t **buf,
uint8_t *len,
uint16_t *len,
uint8_t *name_hash,
char *name,
uint8_t *name_len
Expand Down Expand Up @@ -313,7 +313,7 @@ cmp_name(
static inline bool
parse_resource_len(
volatile uint8_t **buf,
uint8_t *len,
uint16_t *len,
hns_varint_t *ctr,
ledger_blake2b_ctx *hash
) {
Expand Down Expand Up @@ -346,7 +346,7 @@ parse_resource_len(
static inline bool
parse_resource(
volatile uint8_t **buf,
uint8_t *len,
uint16_t *len,
hns_varint_t *ctr,
ledger_blake2b_ctx *hash
) {
Expand All @@ -365,6 +365,7 @@ parse_resource(
if (*ctr > 0) {
if (*len != 0)
THROW(HNS_INCORRECT_PARSER_STATE);

return false;
}
}
Expand All @@ -391,7 +392,7 @@ parse_resource(
static inline uint8_t
parse(
uint8_t p1,
uint8_t *len,
uint16_t *len,
volatile uint8_t *buf,
volatile uint8_t *res,
volatile uint8_t *flags
Expand Down Expand Up @@ -987,7 +988,7 @@ parse(
static inline uint8_t
sign(
uint8_t p1,
uint8_t *len,
uint16_t *len,
volatile uint8_t *buf,
volatile uint8_t *sig,
volatile uint8_t *flags
Expand Down Expand Up @@ -1239,11 +1240,11 @@ sign(
return 65;
}

uint8_t
uint16_t
hns_apdu_get_input_signature(
uint8_t p1,
uint8_t p2,
uint8_t len,
uint16_t len,
volatile uint8_t *in,
volatile uint8_t *out,
volatile uint8_t *flags
Expand Down
8 changes: 4 additions & 4 deletions src/apdu.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -295,11 +295,11 @@ hns_apdu_get_app_version(
* @return the status word
*/

uint8_t
uint16_t
hns_apdu_get_public_key(
uint8_t p1,
uint8_t p2,
uint8_t len,
uint16_t len,
volatile uint8_t *in,
volatile uint8_t *out,
volatile uint8_t *flags
Expand All @@ -320,11 +320,11 @@ hns_apdu_get_public_key(
* @return the status word
*/

uint8_t
uint16_t
hns_apdu_get_input_signature(
uint8_t p1,
uint8_t p2,
uint8_t len,
uint16_t len,
volatile uint8_t *in,
volatile uint8_t *out,
volatile uint8_t *flags
Expand Down
4 changes: 2 additions & 2 deletions src/ledger.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,11 +119,11 @@ ledger_apdu_cache_write(volatile uint8_t *src, uint8_t src_len) {
}

uint8_t
ledger_apdu_cache_flush(uint8_t *len) {
ledger_apdu_cache_flush(uint16_t *len) {
uint8_t *cache = g_ledger_apdu_cache;
uint8_t *buffer = g_ledger_apdu_buffer;
uint8_t cache_len = g_ledger_apdu_cache_len;
uint8_t buffer_len = 0;
uint16_t buffer_len = 0;

if (cache_len == 0)
return 0;
Expand Down
2 changes: 1 addition & 1 deletion src/ledger.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ ledger_apdu_cache_write(volatile uint8_t *src, uint8_t src_len);
* @return the amount of data added to the exchange buffer from the cache.
*/
uint8_t
ledger_apdu_cache_flush(uint8_t *len);
ledger_apdu_cache_flush(uint16_t *len);

/**
* Checks the apdu cache buffer for stored data.
Expand Down
19 changes: 10 additions & 9 deletions src/utils.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ size_varsize(size_t val) {
}

static inline bool
read_u8(volatile uint8_t **buf, uint8_t *len, uint8_t *u8) {
read_u8(volatile uint8_t **buf, uint16_t *len, uint8_t *u8) {
if (*len < 1)
return false;

Expand All @@ -251,7 +251,7 @@ read_u8(volatile uint8_t **buf, uint8_t *len, uint8_t *u8) {
}

static inline bool
read_u16(volatile uint8_t **buf, uint8_t *len, uint16_t *u16, bool be) {
read_u16(volatile uint8_t **buf, uint16_t *len, uint16_t *u16, bool be) {
if (*len < 2)
return false;

Expand All @@ -270,7 +270,7 @@ read_u16(volatile uint8_t **buf, uint8_t *len, uint16_t *u16, bool be) {
}

static inline bool
read_u32(volatile uint8_t **buf, uint8_t *len, uint32_t *u32, bool be) {
read_u32(volatile uint8_t **buf, uint16_t *len, uint32_t *u32, bool be) {
if (*len < 4)
return false;

Expand All @@ -291,7 +291,7 @@ read_u32(volatile uint8_t **buf, uint8_t *len, uint32_t *u32, bool be) {
}

static inline bool
read_varint(volatile uint8_t **buf, uint8_t *len, hns_varint_t *varint) {
read_varint(volatile uint8_t **buf, uint16_t *len, hns_varint_t *varint) {
if (*len < 1)
return false;

Expand Down Expand Up @@ -350,7 +350,7 @@ read_varint(volatile uint8_t **buf, uint8_t *len, hns_varint_t *varint) {
}

static inline bool
peek_varint(volatile uint8_t **buf, uint8_t *len, hns_varint_t *varint) {
peek_varint(volatile uint8_t **buf, uint16_t *len, hns_varint_t *varint) {
if (!read_varint(buf, len, varint))
return false;

Expand All @@ -363,7 +363,7 @@ peek_varint(volatile uint8_t **buf, uint8_t *len, hns_varint_t *varint) {
}

static inline bool
read_varsize(volatile uint8_t **buf, uint8_t *len, size_t *val) {
read_varsize(volatile uint8_t **buf, uint16_t *len, size_t *val) {
hns_varint_t v;

if (!read_varint(buf, len, &v))
Expand All @@ -375,7 +375,7 @@ read_varsize(volatile uint8_t **buf, uint8_t *len, size_t *val) {
}

static inline bool
read_bytes(volatile uint8_t **buf, uint8_t *len, volatile uint8_t *out, size_t sz) {
read_bytes(volatile uint8_t **buf, uint16_t *len, volatile uint8_t *out, size_t sz) {
if (*len < sz)
return false;

Expand All @@ -390,7 +390,7 @@ read_bytes(volatile uint8_t **buf, uint8_t *len, volatile uint8_t *out, size_t s
static inline bool
read_varbytes(
volatile uint8_t **buf,
uint8_t *len,
uint16_t *len,
uint8_t *out,
size_t out_sz,
size_t *out_len
Expand All @@ -412,6 +412,7 @@ read_varbytes(
if (!read_bytes(buf, len, out, sz)) {
*buf -= offset;
*len += offset;

return false;
}

Expand All @@ -423,7 +424,7 @@ read_varbytes(
static inline bool
read_bip44_path(
volatile uint8_t **buf,
uint8_t *len,
uint16_t *len,
uint8_t *depth,
uint32_t *path,
uint8_t *info
Expand Down

0 comments on commit a4ac01e

Please sign in to comment.