Out of bounds reads in OpenSSH and some lessons learned about vulnerability terminology
OpenSSH 6.9 has been released yesterday and fixes two out of bounds read issues I had reported.
The first one is not very interesting. A null byte in a configuration file can cause an off-by-one read access. As a config file is not attacker controlled this doesn't have any security impact.
The second issue is an out of bounds read access in the function match_pattern_list() through the ssh handshake. I found this while experimenting with a way to do network fuzzing with american fuzzy lop via preloading a library that will intercept networking functions. I hope to be able to release the tool I use for it at some point, right now it's not really in a usable state. (I later learned that there is a similar tool called Preeny.)
The match_pattern_list() function had two strings and the length of the second string as parameters. The length of the first string was determined by strlen(). A call in the file combat.c called that function with the length of the first string as the parameter, causing an out of bounds read.
This issue can be reproduced by:
a) Compiling openssh 6.8p1 with address sanitizer (./configure CFLAGS="-fsanitize=address" LDFLAGS="-fsanitize=address"; make)
b) Running this netcat-command:
echo "SSH-2.0-OpenSSH_6.5"|nc -l -p 22
c) Connecting to it: ./ssh 127.0.0.1
OpenSSH fixed this by changing the function match_pattern_list() to accept only the two strings and checking both lengths with strlen().
Now there's a backstory to this. When I reported this to Damien Miller he considered it not security relevant and published the fix right away to the public git. It wasn't noticed by anyone until I briefly mentioned it in a post to the american fuzzy lop users mailing list. People started linking to this post on twitter so I thought I should explain it more detailed and wrote a message to oss-security. That only caused more (probably undeserved) attention, up to the point where some people would call it a high risk vulnerability.
Some people criticised me for calling this issue a heap overflow. This highlights a problem: The term buffer overflow and the related terms stack / heap overflow aren't defined in a strict way. Some consider only out of bounds writes an overflow (e. g. the CWE-120 definition), others also call out of bounds reads an overflow (e. g. Address Sanitizer). See also David Wheeler's discussion on Heartbleed. To avoid any confusion I decided for myself to use the term out of bounds read in the future for similar issues.
There are two takeaways for me from this story:
a) If you write about potential vulnerabilities in important security software it's hard to avoid that others will exaggerate the issue.
b) Vulnerability terminology can be ambiguous and is not always strictly defined.
The first one is not very interesting. A null byte in a configuration file can cause an off-by-one read access. As a config file is not attacker controlled this doesn't have any security impact.
The second issue is an out of bounds read access in the function match_pattern_list() through the ssh handshake. I found this while experimenting with a way to do network fuzzing with american fuzzy lop via preloading a library that will intercept networking functions. I hope to be able to release the tool I use for it at some point, right now it's not really in a usable state. (I later learned that there is a similar tool called Preeny.)
The match_pattern_list() function had two strings and the length of the second string as parameters. The length of the first string was determined by strlen(). A call in the file combat.c called that function with the length of the first string as the parameter, causing an out of bounds read.
This issue can be reproduced by:
a) Compiling openssh 6.8p1 with address sanitizer (./configure CFLAGS="-fsanitize=address" LDFLAGS="-fsanitize=address"; make)
b) Running this netcat-command:
echo "SSH-2.0-OpenSSH_6.5"|nc -l -p 22
c) Connecting to it: ./ssh 127.0.0.1
OpenSSH fixed this by changing the function match_pattern_list() to accept only the two strings and checking both lengths with strlen().
Now there's a backstory to this. When I reported this to Damien Miller he considered it not security relevant and published the fix right away to the public git. It wasn't noticed by anyone until I briefly mentioned it in a post to the american fuzzy lop users mailing list. People started linking to this post on twitter so I thought I should explain it more detailed and wrote a message to oss-security. That only caused more (probably undeserved) attention, up to the point where some people would call it a high risk vulnerability.
Some people criticised me for calling this issue a heap overflow. This highlights a problem: The term buffer overflow and the related terms stack / heap overflow aren't defined in a strict way. Some consider only out of bounds writes an overflow (e. g. the CWE-120 definition), others also call out of bounds reads an overflow (e. g. Address Sanitizer). See also David Wheeler's discussion on Heartbleed. To avoid any confusion I decided for myself to use the term out of bounds read in the future for similar issues.
There are two takeaways for me from this story:
a) If you write about potential vulnerabilities in important security software it's hard to avoid that others will exaggerate the issue.
b) Vulnerability terminology can be ambiguous and is not always strictly defined.