Finding integer overflows in libssh2

September 10, 2019


Reading time

Video Transcription

Hi, my name is Kevin Backhouse. I'm a security researcher at Semmle. This video is about an integer overflow bug that can cause error conditions to get accidentally ignored. I'm going to show you how to write a query that finds variants of that bug.

The query that's from a blog post which I wrote recently about libssh2 (you can see it here if you scroll down in the blog post) is not the query that found the CVE which the blog post is mainly about. While we were looking at the source code for libssh2, we noticed another bug which had been introduced on their development branch -- the good news is that this query was able to find all the variants of it before they released a new version of libssh2.

Let me just quickly show you what the bug is. You've got a call to this function _libssh2_get_c_string and you can see that it sometimes returns a negative number as an error code. You can see that the type of p_len, which it's assigned to, is an unsigned long. That means that if the function returns an error code, then it will trigger an integer overflow here and this less-than-zero condition will never fail. The error code will get ignored which is probably not a good thing.

Let me show you how we can write a query to find that kind of bug. First of all, we're going to write a very simple query to find return statements that return a negative number. Most of these are not going to be bugs, but we're going to make the query more sophisticated.

There's an example where we're returning minus one as an error code. What we're going to do now is connect that to the call where that function is called from. What I've done here is I've added a function f and a call, and I've said that the target of the call is f and the return statement is in the function that we're calling. If you run that, you can see that we've now matched up return statements with where it's called from.

Here we've got a return of minus one, in a function called netconf_read_until, and that's where it's called from there. This one you can see is being handled correctly because there's a good check there and there's no problem. What we're going to do now is look for cases where that call is cast to an unsigned type. I've added this extra line of code here, and what it's saying is that the fully converted type of a call.

In C, often you have both explicit casts and implicit casts. An awful lot of the time the C compiler is inserting implicit casts for you and what getFullyConverted does is it gets you the end of the chain of casts, so you know what type it's going to get converted to. We're asking for the fully converted expression, getting the type, and then getUnderlyingType strips off things like typedefs. We find out what the type really is rather than the typedef, and we say that we're only interested if that type is unsigned.

If we run that query, we can see now we've got 22 results. These are good results that we're finding here. You see in this example we've got a call to that function that we saw before,_get_c_string, and it's getting assigned to r_len, which is an unsigned int. That is an example of the bug.

Now, the query that I wrote for the blog post is just slightly more sophisticated than what I just showed you here. In the next iteration of the query, what I'm going to do is handle cases like this example. In the previous version of the query, I was only looking for cases where the result of the function is immediately converted to an unsigned. But what sometimes will happen is that you'll assign it to a signed integer and then sometime later on that gets converted to an unsigned int.

Now, I've imported our data flow library and then I've said that there's a source and a sink. The source is the function call and the sink is the thing that gets converted to unsigned. Let's see what happens when we run that query.

Now, what we see here is that some of these results are not very good. This one is kind of weird. We've got a call here and it's saying that the assignment expression is a sink. The mistake that I've made in my query, is that I have data flow from the function to anywhere where the type of the sink is unsigned and that's not what I want. What I actually want, is to find the place where the transition happens from signed to unsigned, and right now I'm not restricting it to just those places in the query.

In the final iteration of the query, I'm going to add one more condition. What I've done here is import the range analysis library and added one extra condition, which is that the lower bound of the sink must be negative. What I'm saying here is that the sink itself should be potentially a negative value, but it gets converted to something that is unsigned. The reason why I'm using range analysis to do this less-than-zero check, rather than just saying the type of sink.asExpr() is signed, is that it allows you to handle slightly more sophisticated cases like this example here, where you've got an if statement that does a bounds check on the value of r. Range analysis will know that at this point where you use r, it is definitely a positive number and the query won't give you a false positive result. It just makes the quality of a query slightly higher. If we run this query now, we can see that we just have 13 good results. These are all true positive results that we get here.

One final thing to mention is if you are working on a software project where you use negative numbers as error codes, then I really recommend that you give this query a try. In the blog post, there's a link to the LGTM query console which shows you the results on libssh2. If your project is open source, you can grab this query and try it out on your own project to see if you've made this mistake in your project.