[CPP][Linux Kernel]Add ql to detect CVE-2017-5123

[4B5F5F4B]

No description provided.

[MathiasVP]

Hi @4B5F5F4B,

Thanks for your contribution! We'll get to review it soon, but in the meantime would you mind restructuring your PR so that your query is in the https://github.com/github/codeql/tree/main/cpp/ql/src/experimental/Security/CWE directory? I guess this query is an instance of CWE-020, right? So I think it would be good to put the query into the https://github.com/github/codeql/tree/main/cpp/ql/src/experimental/Security/CWE/CWE-020 directory.

You can move the existing code and tests in that folder into a new directory (maybe called LateCheckOfFunctionArgument or something) if you want.

[rdmarsh2]

Minor typo - Wrtie for Write

[rdmarsh2]

Thanks for the contribution! This looks good as-is other than the typo and the folder (as Mathias mentioned, we arrange queries by CWE rather than CVE). If you want to extend it further, you might look at the global data flow library and the BarrierGuard class to get a more precise interprocedural version of the query.

[4B5F5F4B]

Thanks for the contribution! This looks good as-is other than the typo and the folder (as Mathias mentioned, we arrange queries by CWE rather than CVE). If you want to extend it further, you might look at the global data flow library and the BarrierGuard class to get a more precise interprocedural version of the query.

Hello, @rdmarsh2
I'll try to solve all your concerns as soon as possible.

[4B5F5F4B]

Hello, @rdmarsh2

I've moved and renamed my query under experimental/security/CWE directory just as @MathiasVP suggested before, what's more I modified the typo you spotted:)

Thanks a lot for your suggestion that BarrierGuard and global data flow library should be used, but frankly speaking as far as bugs like CVE-2017-5123 is concerned, intraprocedural analsis maybe good enough. Because security check for user-mode pointer in linux kernel codebase is not a function but marco, it's not common that a user mode pointer is checked by access_ok in function a, and passed to unsafe_put_user in function b.

[rdmarsh2]

There's one more change I have to ask for, which is causing the test failure - for this to be an @kind problem query, it needs to have an even number of columns, alternating between locations and string (see this page on defining query results), The easy fix is to just remove p from the select, or you could use the $@ syntax to include the name of the parameter as well.

[4B5F5F4B]

Hello, @rdmarsh2
I Just modified the select clause to make CodeQL happy, please have a look. thx

[rdmarsh2]

Did you want the parameter or the unsafe_put_user invocation to be the alert location? After your change, it's the parameter, but I'd expect it to be the macro invocation. Looks good otherwise.

[4B5F5F4B]

Hello @rdmarsh2

After carefully consideration I prefer parameter to be the alert location, because the intention of my query is to detect vulnerability pattern that function's parameter abused in unsafe_put_user, and there maybe multiple unsafe_put_user invocations that abuse the same parameter in one function.

for example, consider the following buggy code

function vuln(struct something* __user ptr)
{
      unsafe_put_user(0x41414141,   ptr->foo, 4);
      unsafe_put_user(0x42424242, ptr->bar, 4);
  
}

[4B5F5F4B]

Hello @rdmarsh2

I just refactor some code, and make pull request here #8599, please take a look, thx:)