12 Mar 2024
I was just checking my mail today when I found a new issue filed in picoquic: Stack-based buffer overflow in picoquic_verify_retry_token. Oh well, time to eat some humble pie. The issue happens in the function that decodes the “Retry Token” - see Section 8.1.2 of RFC 9000. This function tries to decrypt a retry token into uint8_t text[128]. This token is received from network, so it could be crafted by a malicious actor. There’s no explicit restriction on its length, so there’s no guarantee the result will fit into the buffer. The length of the buffer is not passed on from this function, the decryption function just assumes it is big enough. This could result in stack overwrite with potential exploit implications, or denial of service.
This was easily fixed, but it shows a couple of interesting issues. First, a classic failure to check an external input. When I wrote that code, I was assuming that the clients would only send the tokens provided by the proper server, and that in any case, since the tokens were encrypted, any tempering would result in a decryption error and cause the token to be rejected. But the particular decryption procedure that I use will overflow if the decryption buffer is too small. The fix was to test that.
The second issue is that this should have been found in testing. Picoquic has extensive tests, including fuzzing of initial packets. But the fuzzing process was not programmed to create tokens of arbitrary sizes, so the bug was not detected.
Well, nobody is perfect…
If you want to start or join a discussion on this post, the simplest way is to send a toot on the Fediverse/Mastodon to @huitema@social.secret-wg.org.