CVE-2021-22898: TELNET stack contents disclosure (#1176461) issue was recently reported for curl and it was addressed in curl 7.77.0:
https://curl.se/docs/CVE-2021-22898.html
https://github.com/curl/curl/commit/39ce47f219b09c380b81f89fe54ac586c8db6bde
https://hackerone.com/reports/1176461
However, the fix applied is not correct and does not completely address the issue. It helps in cases when long environment variable name is used ('a'*256 + ',b'
), but not when the name is short and only the value is long ('a,' + 'b'*256
, which is the example mentioned in the curl project advisory).
Follow the steps form #1176461, only use NEW_ENV option with short name and long value, such as:
$ curl telnet://127.0.0.1:23 -t NEW_ENV=`python -c "print('a,' + 'b'*256)"`
When parsing NEW_ENV option value with short name and long value, sscanf() returns 2, as it writes to both varname
and varval
, even though the data in varval
is truncated. Hence such variable is not skipped and is added to the temp[]
buffer. However, the len
counter which tracks the amount of data that was already written to temp[]
is not updated based on the data written to the buffer in the msnprintf()
call, but rather based on the length of the original unparsed data that is stored in tmplen
. The relevant code is here:
https://github.com/curl/curl/blob/curl-7_77_0/lib/telnet.c#L926-L929
When value stored in varval
is truncated, len
is increased too much and a chunk of uninitialized memory is created in temp[]
. The len
should only be incremented by strlen(varname) + strlen(varval) + 2
.
I wonder if the original fix should be preserved or re-worked. In addition to not fixing the info leak problem properly, it also causes certain valid option values to be ignored and not sent to a server any more. Rejected values are of the forms NEW_ENV=a
or NEW_ENV=a,
. At least the second one seems like an obviously valid way to set variable a
to an empty string. RFC 1572 defines that environment variable can be sent with empty value and hence NEW_ENV=a,
should remain supported. It also defines that variable can be sent with no value, making NEW_ENV=a
a valid option as well. Note that curl prior to 7.77.0 actually did handle NEW_ENV=a
that way, but it looks more like an unintended side effect of how len
was incremented by tmplen
, as the empty value part was written to temp[]
and only subsequently overwritten. As the telnet protocol support in curl is not likely to be used widely these days, possibly only to interact with some legacy systems, it seems reasonable to prefer a fix that changes behaviour as little as possible.
Leak of an uninitialized stack memory.
Report #1176461 and the matching curl advisory provide some estimates on how much data can be leaked. I believe the amount of leaked data is smaller and is less than a half of the temp[]
size. The reason for that is in the check_telnet_options()
where option arguments are truncated to 255 characters, and at least half of that must part of the defined variable name or value.
https://github.com/curl/curl/blob/curl-7_77_0/lib/telnet.c#L799-L800