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
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
dd a0 19 data is not allowed by the specification and has no meaning in the protocol.
So how did this extra data segment get there?
I’m a freelance Security Consultant and currently available for new projects. If you are looking for assistance to secure your projects or organization, contact me.
In the U2F HID protocol, every connection is initiated by the host via an
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
- the specific response packet section
U2FHID_INIT_RESPwhich is used as an element of
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:
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:
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. Consequently, 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
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.
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.
On the Trezor One and other hardware wallet devices, sensitive stack variables are supposed to be overwritten with
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.
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.
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
- Best practice: if structs are directly used in packet handling, explicitly mark them as __packed__
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.
|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|
|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|
SatoshiLabs, ShapeShift and Shift Cryptosecurity provided a bug bounty.
I would like to thank Dr. Jochen Hoenicke for his help in identifying the underlying struct issue.
- Many of the relevant technical aspects and implications are described in the Trezor blog article on the issue that I have co-written.
- Formal description of this vulnerability class.
- I have found at least one software verification framework that does not model the C struct behavior correctly.
- The Lost Art of Structure Packing
- Data structure alignment
Additional technical information
For readers who are interested in more low-level details, here is the original packet again with markers:
ff ff ff ff 86 00 11 6b 48 15 5b b6 ea f8 fe c00050 33 36 f6 02 01 06 01 01dd a0 19 00 00 00 00 00
The relevant specifications of the valid packet:
||channel identifier that references the BROADCAST packet by the host|
||payload size, upper byte|
||payload size, lower byte|
|U2F payload||size (Byte)||content||comment|
|nonce||8||chosen by the host|
|cid||4||chosen by the device|
|versionMajor||1||firmware version digit||e.g.
|versionMinor||1||firmware version digit||e.g.
|versionBuild||1||firmware version digit||e.g.