In November 2018, I noticed irregularities during packet analysis of U2F packets on the Trezor One hardware wallet. The data segment in the U2F init response packets was three bytes longer than it should be. According to the U2F specification, this packet consists of 24 (= 7 + 17) data bytes and the rest of the 64 byte USB packet must be set to 0x00.

However, the data on the wire looked like this:

0040   ff ff ff ff 86 00 11 6b 48 15 5b b6 ea f8 fe c0
0050   33 36 f6 02 01 06 01 01 dd a0 19 00 00 00 00 00
0060   00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0070   00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00

The trailing dd a0 19 are not allowed by the specification and have no meaning in the protocol.

So how did this extra data get there?

The vulnerability

In the U2F HID protocol, every connection is initiated by the host via a U2FHID_INIT packet. The U2F device assigns a new channel ID and answers with a response packet that also contains some U2F protocol flags as well as a few bytes of other device information. The problematic bytes are in this response packet.

The C code in the relevant open source projects handles this via two C structs:

  • the general U2F HID packet structure U2FHID_FRAME
  • the specific response packet section U2FHID_INIT_RESP which is used as an element of U2FHID_FRAME

This logic and naming follows the “upstream” Yubico u2f_hid.h reference code which is part of the U2F specification.

At first, the relevant code section of the Trezor One looked harmless.
This is the struct definition for the inner part of the message:

  typedef struct
  {
    uint8_t nonce[INIT_NONCE_SIZE];
    uint32_t cid;
    uint8_t versionInterface;
    uint8_t versionMajor;
    uint8_t versionMinor;
    uint8_t versionBuild;
    uint8_t capFlags;
  } U2FHID_INIT_RESP;

In the Trezor code, the complete U2FHID_FRAME struct is overwritten with 0x00 during initialization and all fields of the U2FHID_INIT_RESP struct are overwritten before it is copied there:

    U2FHID_FRAME f;
    U2FHID_INIT_RESP resp;
[...]
    memset(&f, 0, sizeof(f));
[...]
    memcpy(resp.nonce, init_req->nonce, sizeof(init_req->nonce));
    resp.cid = in->cid == CID_BROADCAST ? next_cid() : in->cid;
    resp.versionInterface = U2FHID_IF_VERSION;
    resp.versionMajor = MAJOR_VERSION;
    resp.versionMinor = MINOR_VERSION;
    resp.versionBuild = PATCH_VERSION;
    resp.capFlags = CAPFLAG_WINK;
    memcpy(&f.init.data, &resp, sizeof(resp));

When looking at this C code, one notices that resp is not initialized explicitly with zeros. However, isn’t overwriting each individual field of the struct with new data “good enough”?

As it turns out, it isn’t.

After closer inspection, we figured out that the U2FHID_INIT_RESP resp struct actually contained a hidden field since C structs are per default considered as unpacked structs, which (simplified) means that the compiler can add padding between or after the struct fields if it is beneficial, e.g. for better memory access on some alignment boundary. The specific combination of fields in the U2FHID_INIT_RESP struct matched the conditions.

As a result, the memory representation of the struct differs from the one that is expected, which is highly problematic for a struct that is meant to encode data “on the wire”.
In this particular case, there are three bytes of padding at the end of the structure. Consequentially, the sizeof(resp) is 20 in the code above instead of the expected 17, which means the padding is copied to f as well during the memcpy() call. This is where the extra data in the USB capture shown in the beginning comes from.

Notably, the exact position of the padding at the end of the struct made it easier to miss during development. Other padding positions would have caused more obviously defective U2F packets through offsets.

Issue summary

The relevant resp struct is initialized with previously used stack memory, only partially overwritten (first 17 bytes) and then copied in full (20 bytes) to the USB bus via memcpy(&f.init.data, &resp, sizeof(resp)); from where it reaches the host. For a device which holds and handles secrets, this is a security problem.

Security implications, attack scenario

A local attacker on the host gets three bytes of uninitialized stack memory from the U2F device without authentication on every valid U2F channel initialization handshake. No unusual or malformed request packets are necessary.

The leaked memory section is highly device- and firmware-specific. The U2F initialization can be repeated a large number of times without any counter-, PIN- or other check.

Existing countermeasures

On the Trezor One and other hardware wallet devices, sensitive stack variables are supposed to be overwritten with 0x00 via memzero() after their use so that they can’t be leaked through a vulnerability like this. In practice, it is not straightforward to guarantee that this memory sanitation is working in all relevant cases. In part this is due to the known problem of compiler optimizations undermining those efforts (as explained in the Memsad talk by Ilja van Sprundel) and the fact that a silent failure will not be noticed.

Many operations on a cryptocurrency hardware wallet are designed to be inaccessible until the device owner has unlocked the device through a PIN or passphrase. However, since the U2F design does not include such a user-provided device secret and U2F is meant to be usable directly after device power-on, all of the relevant code paths are reachable regardless of the device unlock state.

Remote attacks

Although U2F is reachable for remote attackers via the browser and the affected devices will actually leak stack memory in the response packet, the response bytes will (to my knowledge) not be exposed back to the attacker via the browser U2F API since this section of the raw packet will be discarded. The issue is therefore limited to local attackers.

Mitigations

There are a number of ways to correct the memory representation of the struct.

The cleanest approach is to declare the relevant struct as packed to make it behave as originally intended by specifying __attribute__((__packed__)) in the struct definition. As it turned out, newer versions of the header files by Yubico / the FIDO Alliance already have this fix in place, but there are many old copies.

The compiler flag -Wpadded will add warnings if there are padded structs present, which may help during development to notice this issue.

Lessons learned - development perspective

  • Reference implementations can have flaws in them
  • Best practice: manually overwrite all your structs with 0x00 during initialization
  • Best practice: if structs are directly used in packet handling, explicitly mark them as __packed__

Disclosure coordination

Due to the open source nature of the Yubico header files and forks of the Trezor One U2F implementation, the issue was present in multiple products. I’ve analyzed a number of projects for their susceptibility to this attack and responsibly disclosed the issue to the affected vendors where it was necessary.

Overall, this disclosure schedule was very fast and we released the information publicly only three weeks after the initial discovery, shortly before the Christmas holidays.

Relevant projects

product fixed fw version vendor article notes
SatoshiLabs Trezor One 1.7.2 Trezor article  
ShapeShift KeepKey 5.10.3 Shapeshift article VULN-1861, Note: U2F was disabled on previous firmware versions due to unrelated issues.
Shift Cryptosecurity BitBox01 6.0.0 Shift article  
Spark Innovations SC4-HSM - Article Note: no practical impact, as noted during the initial disclosure

Detailed timeline

Date info
2018-11-26 Discovery of U2FHID_INIT_RESP information leak
2018-11-26 Disclosure to SatoshiLabs
2018-11-30 Advance notice to ShapeShift
2018-12-05 Disclosure to ShapeShift
2018-12-12 Disclosure to Shift Cryptosecurity
2018-12-15 Disclosure to Spark Innovations
2018-12-18 12:00 CET Coordinated public disclosure

Bug bounty

SatoshiLabs, ShapeShift and Shift Cryptosecurity provided a bug bounty.

Credits

I would like to thank Dr. Jochen Hoenicke for his help in identifying the underlying struct issue.

Further references

Additional technical information

Packet details

For readers who are interested in more low-level details, here is the original packet again with markers:

0040   ff ff ff ff 86 00 11 6b 48 15 5b b6 ea f8 fe c0
0050   33 36 f6 02 01 06 01 01 dd a0 19 00 00 00 00 00

The relevant specifications of the valid packet:

Field size (Byte) content function
CID 4 0xffffffff channel identifier that references the BROADCAST packet by the host
CMD 1 0x86 command id
BCNTH 1 0x00 payload size, upper byte
BCNTL 1 0x11 payload size, lower byte
U2F payload size (Byte) content comment
nonce 8 chosen by the host  
cid 4 chosen by the device  
versionInterface 1 0x02 U2F version
versionMajor 1 firmware version digit e.g. 0x01
versionMinor 1 firmware version digit e.g. 0x06
versionBuild 1 firmware version digit e.g. 0x01
capability flag 1 device-specific usually 0x01 due to CAPFLAG_WINK