Back to GitHub.com
Home Bounties Research Advisories CodeQL Wall of Fame Get Involved Events
January 24, 2018

Stack buffer overflow in Qualcomm MSM 4.4 - Finding bugs with CodeQL

Kevin Backhouse

In this post, I will describe a simple query for C programs that can be used to detect an important class of bugs in operating system kernels. The query found a stack buffer overflow bug in the Qualcomm MSM-4.4 Android kernel. Bugs like this should never be allowed to get past code review, so I will show how you can use CodeQL to automatically detect them.

About ‘copy_from_user’

The MSM-4.4 kernel uses a function named copy_from_user to copy data from user space into kernel space. copy_from_user is typically used to copy data from user to kernel space when pointers to user space memory are passed to a system call (directly or indirectly). This works as follows:

copy_from_user needs to be used with extreme care to avoid creating a security flaw in the kernel. In particular, it is crucial that you allocate a sufficiently large buffer in the kernel to receive the data. Otherwise, an attacker could overflow the buffer by passing a large size value to the system call.

Using CodeQL to find stack buffer overflows

Perhaps the most obvious mistake that you could make with copy_from_user is to use it to copy data directly onto the kernel’s stack without a bounds check on the size parameter. This would enable an attacker to overwrite the kernel stack, which means that they could change the return address of the current function and execute arbitrary code. I wrote the following query to search for such bugs:

import cpp
import semmle.code.cpp.dataflow.StackAddress

from FunctionCall call, Expr destArg, Expr sizeArg
where call.getTarget().getName() = "copy_from_user"
and destArg = call.getArgument(0)
and sizeArg = call.getArgument(2)
and stackPointerFlowsToUse(destArg, _, _, _)
and not sizeArg.isConstant()
select call

In words, this query looks for calls to copy_from_user, where argument 0 (the destination buffer) is a stack address, and argument 2 (the size) is not a constant.

This query is very simple because I was able to use the stackPointerFlowsToUse predicate from the StackAddress CodeQL library to determine whether argument 0 contains a stack address. The StackAddress library uses a sophisticated dataflow analysis to track stack pointers through the program. With libraries like this, you can write very powerful queries with just a few lines of QL.

The condition that the size argument should not be a constant is a bit simplistic, because it does not exclude safe code like this:

char str[MAX_MSG_LEN];
u32 size = sizeof(str) < count ? sizeof(str) : count;
memset(str, 0, sizeof(str));
missing = copy_from_user(str, buf, size);

Even though count contains an untrusted value, this code is safe because size is guaranteed to be bounded by sizeof(str). It is easy to remove false positives like this from the list of results by adding another clause to the query:

import cpp
import semmle.code.cpp.dataflow.StackAddress
import semmle.code.cpp.rangeanalysis.SimpleRangeAnalysis

from FunctionCall call, Expr destArg, Expr sizeArg
where call.getTarget().getName() = "copy_from_user"
and destArg = call.getArgument(0)
and sizeArg = call.getArgument(2)
and stackPointerFlowsToUse(destArg, _, _, _)
and not sizeArg.isConstant()
and not exists (LocalScopeVariable v, int destSize
        | destArg = v.getAnAccess() and
          destSize = v.getType().getSize() and
          0 <= lowerBound(sizeArg) and
          upperBound(sizeArg) <= destSize)
select call

The new clause excludes cases where the destination buffer is a local variable and the size argument is bounded by the size of the buffer. I used the SimpleRangeAnalysis CodeQL library to compute bounds for sizeArg. Range analysis also involves a non-trivial dataflow analysis, so this is another example of how you can use the CodeQL libraries to write sophisticated analyses with just a few lines of QL.

The bug

One of the results of the query is msm_cpp.c, line 2863:

static int msm_cpp_copy_from_ioctl_ptr(void *dst_ptr,
  struct msm_camera_v4l2_ioctl_t *ioctl_ptr)
{
  ...
  ret = copy_from_user(dst_ptr,
    (void __user *)ioctl_ptr->ioctl_ptr, ioctl_ptr->len);
  ...
}

The query has detected that dst_ptr sometimes contains a stack address. However, it is a parameter so we need to look at the call sites of msm_cpp_copy_from_ioctl_ptr to determine whether this is a real bug. There are 7 call sites in total. Most of these call sites are safe because they check the bounds of ioctl_ptr->len (which contains an untrusted value), like this. But two of the calls sites, msm_cpp.c:3507 and msm_cpp.c:3545, have no such check, which means that an attacker could overwrite the stack.

Bug fix and other mitigations

This bug was present in the published version of MSM-4.4 which I downloaded from source.codeaurora.org on May 14, 2017. At the time, the published version of branch rel/msm-4.4 was revision 1ccabf65ac2. The bug was still present when I reported it to Qualcomm on May 28, 2017. The fixed version of the code was published on June 1, 2017. It turned out that Qualcomm had already fixed the bug internally (in revision 0f4c6731385), but had not yet published it when I reported it to them. It is unclear from the commit message whether the developer who fixed the bug was aware that they were fixing a security vulnerability. My guess is that they thought it was just a normal bug fix, which is why the bug was never assigned a CVE or mentioned in a security advisory.

The bug was originally introduced on October 13, 2016 (in revision d70b369d3ee). I do not know when the bug was first published on source.codeaurora.org, but it seems likely that it was in the official release for about 6 months.

The bug is difficult to exploit by itself because the Android kernel is protected by other security layers. In particular, an exploit would need to be able to open the file /dev/v4l-subdev0, which is typically only accessible by the system user, and call ioctl on it. However, if an attacker has already succesfully gained system privileges through a separate vulnerability, then they can leverage this bug to execute arbitrary code in the kernel.

Conclusion

I have shown how to write a simple query that detects an important type of kernel stack overflow vulnerability. The query found a real bug in Qualcomm’s MSM-4.4 kernel. It turned out that Qualcomm had already fixed the bug, but they had not yet published the fix when I reported it to them. The bug was never assigned a CVE, but I believe it was present in the published version of MSM-4.4 for several months prior to June 1, 2017.

Bug timeline

Note: Post originally published on LGTM.com on January 24, 2018