I have discovered two new security issues in the Yubico libykpiv client-side code which were introduced as a regression in the 2.3.0 release.
Flaws in the memory handling of the auth handshake procedure with a PIV smartcard could lead to memory corruption, denial of service or other unexpected behavior under some conditions. The practical security impact on tested production binaries appears to be limited.
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.
Stack-out-of-bounds-write in ykpiv_authenticate2()
The first issue is a code flaw related to insufficient length restrictions for smartcard-provided data. This issue is similar to previous libykpiv vulnerabilities and again leads to dangerous memory safety issues due to custom low-level memory handling.
The ykpiv_authenticate2() function performs a sequence of interactions with an external PIV smartcard such as a Yubikey 5 device connected via USB for smartcard actions that require authentication. During those steps, the host receives a cryptographic challenge from the smartcard:
While there is an upper bound on the received data that prevents any direct issues, the length recv_len of the reply is also reused for the cryptographic challenge from the host to the smartcard. This becomes an issue if recv_len is particularly large:
The manual memory management via custom pointer advances becomes a liability here, since the upper bound on the recv_len is not sufficiently tied to how much data the apdu struct can hold at this point. As a result, _ykpiv_prng_generate(challenge, challenge_len) can end up writing behind the struct if the unchecked assumptions about the memory sizes are violated:
Since _ykpiv_prng_generate() overwrites the target buffer with random data via OpenSSL’s RAND_DRBG_generate() function, a malicious smartcard that triggers this flaw doesn’t have control over the exact values that are written behind apdu on the stack during the out-of-bounds write, and the values will be different on each execution. This doesn’t mitigate the memory safety issue itself but definitely makes it harder to manipulate the stack data in a controlled way.
Due to the compiler- and target-specific aspects of the program stack layout, it is difficult to make global statements about the expected security implications of the stack-buffer-overflow or lack thereof. By my knowledge, the yubico-piv-tool binary that includes the libykpiv library is always compiled with stack canary protections for production builds, which should turn any out-of-bounds write to the stack canary segment into a controlled program crash and therefore a denial-of-service. Some limited analysis of the situation for Linux x86_64 binaries of the yubico-piv-tool 2.3.0 release suggests that the OOB write can’t reach the stack canary segment and “only” overwrites stack memory of other local variables that are not used at this point.
Given the nature of libykpiv as a library intended for use within other applications, it’s plausible that the affected code is also in use with other build system configurations or compilers where those observations do not apply.
My current understanding is that this flaw cannot be used to hijack the execution flow of the program or manipulate essential internal variables in yubico-piv-tool 2.3.0 and will at worst causes a crash, but my confidence of this is limited due to the outlined complexity. For example, there may be additional variations of this attack by malicious smartcards which are aware of the mgm_key secret that is shared between host and smartcard.
Please note that this scoring assumes that there is a way to impact the availability of the libykpiv component, for example by writing into a segment of the stack memory that is protected by stack canaries or causing a segmentation fault, and that the attacker can get by without authentication secrets. During the disclosure process, we discussed Availability: High vs. Availability: Low impact scoring in such a scenario. Only the lower rating is reflected in the scoring above to accommodate the current uncertainty about practical availability impact.
Stack-use-after-scope in ykpiv_authenticate2()
The second issue is a code flaw related to accessing a C variable’s memory content after its valid lexical program scope.
The ykpiv_authenticate2() function contains multiple code regions with locally scoped variables, as well as variables that are used across multiple regions. Consider the challenge pointer which is defined early in the function:
While the challenge pointer variable itself is valid throughout the ykpiv_authenticate2() function, the stack memory it references has gone out of scope together with the apdu variable at that point. This leads to an AddressSanitizer: stack-use-after-scope error on debug builds with compiler sanitizers. AddressSanitizer warns on the memcmp(data + 4, challenge, challenge_len) call via __interceptor_memcmp, but the cipher_encrypt(mgm_key, challenge, challenge_len, challenge, &out_len) call should be affected by this as well.
There is no security mechanism to detect this in production builds.
In theory, the C compiler is allowed to make arbitrary changes to the referenced stack memory content once it is no longer in scope, e.g., to overwrite it with other variables or clear it. Using this memory again leads to undefined behavior.
The stack-use-after-scope issue is triggered on each successful execution of ykpiv_authenticate2(), but I’m not aware of any bug reports of functional issues in the handshake that are expected if there is a change in memory behavior, and nothing like that has been indicated by Yubico during the disclosure.
Therefore, I think Yubico got lucky with the bug behavior, since the relevant compilers for the production binaries apparently decided to leave the memory content intact long enough so that the logical program execution works as originally intended (likely because it is the fastest behavior). Since I’m not aware of a practical way for an attacker to influence this behavior and use it to their advantage, I’m handling it as a non-issue in terms of practical security impact on the 2.3.0 release.
The most worrying aspect to me is that this bug made it into a stable release without getting detected by static analysis tools or dynamic analysis at runtime despite being present during every smartcard authentication. This suggests that the associated test suites should be improved.
I have the impression that Yubico is currently not assigning a lot of resources or priority to security disclosure handling of their client-side open source libraries. During this coordinated disclosure process, it took almost two months to get a technical reply, and neither a release nor a patch was published during the 90-day disclosure timeframe as far as I’m aware.
The discovered security issues certainly aren’t the most severe, but memory corruption and undefined behavior issues are often difficult to classify as benign with a high certainty due to the amount of compiler- and architecture-related assumptions that may or may not hold in practice for all affected users. Since we didn’t identify a practical security impact on any of the tested production binaries, I decided not to ask for a CVE ID assignment at the moment.
In light of the other difficulties and delays observed during the previous disclosures to Yubico, the current situation is neither very encouraging for researchers who report issues nor adequately reducing the risk to end users via prompt security patches in my opinion.
Relevant yubico-piv-tool / libykpiv Sources
To my knowledge, both regression issues were introduced after the 2.2.0 stable version release tag and are only present in the 2.3.0 stable release.