Skip to content

read HTTPS mandatory SvcParamKeys as 16-bit values#9585

Open
basavaraj-sm05 wants to merge 1 commit into
lysine-dev:masterfrom
basavaraj-sm05:dns-https-mandatory-keys
Open

read HTTPS mandatory SvcParamKeys as 16-bit values#9585
basavaraj-sm05 wants to merge 1 commit into
lysine-dev:masterfrom
basavaraj-sm05:dns-https-mandatory-keys

Conversation

@basavaraj-sm05

Copy link
Copy Markdown

The new DNS reader parses SVCB and HTTPS resource records, and each SvcParam value is length-prefixed. For the mandatory parameter (key 0), RFC 9460 defines the value as a list of 2-octet SvcParamKeys, but readHttpsResourceRecord walks it one byte at a time and range-checks each byte against the supported keys. Since these records come straight off a DNS or DoH response, a resolver can send a mandatory list whose keys OkHttp doesn't support and have it slip through: a two-byte key like 0x0101 gets split into 0x01 and 0x01, both of which look like known keys, so the record is accepted when RFC 9460 says it should be treated as malformed. An odd-length value, which can't be a list of 16-bit keys at all, is likewise read without complaint. I noticed it while checking that every other 2-byte field in this reader already goes through readUShort, and this one call site still used readByte. Reading the keys as 16-bit values and requiring an even length lines the mandatory handling up with the rest of the parser and with the ipv4hint/ipv6hint length checks right next to it. Existing HTTPS records still decode unchanged, and the two new tests cover the unsupported-key and odd-length cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant