CPP: Add query for CWE-476: NULL Pointer Dereference when using exception handling blocks #8245

ihsinme · 2022-02-25T08:12:02Z

this query looks for access situations for pointer data types in situations where they might not have been allocated. the main situation is the transition to the exception handler, before the allocation of memory. an additional query looks for memory re-freeing situations during exception handling.

CVE-2020-15304

I am currently working on making a change to the actual project.

up to head

geoffw0

My initial thoughts. I will need to review the QL in a bit more detail.

cpp/ql/src/experimental/Security/CWE/CWE-476/DangerousUseOfExceptionBlocks.cpp

cpp/ql/src/experimental/Security/CWE/CWE-476/DangerousUseOfExceptionBlocks.ql

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-476/semmle/tests/test.cpp

cpp/ql/src/experimental/Security/CWE/CWE-476/DangerousUseOfExceptionBlocks.cpp

cpp/ql/src/experimental/Security/CWE/CWE-476/DangerousUseOfExceptionBlocks.qhelp

Co-authored-by: Geoffrey White <40627776+geoffw0@users.noreply.github.com>

ihsinme · 2022-03-15T06:07:17Z

Thanks for corrections.

geoffw0

Some suggestions and questions about the query itself.

cpp/ql/src/experimental/Security/CWE/CWE-476/DangerousUseOfExceptionBlocks.ql

geoffw0 · 2022-03-23T19:52:42Z

cpp/ql/src/experimental/Security/CWE/CWE-476/DangerousUseOfExceptionBlocks.ql

+      vro = va.getTarget() and
+      vr != vro
+    ) and
+    pointerDereference(cb, vr, vro) and


My understanding of what's going on here is that we have a try block containing vr = new ..., and a catch block containing delete vr, but its possible to throw before the vr = new ... and reach the delete vr.

What is vro for? Do we really need to track two new allocations to catch this bug?

You are absolutely right if we are talking about deletion.
but I wanted to add detection of a problem with access to unallocated memory, as it happened in CVE-2020-15304.

for (size_t i = 0; i < _data->tileBuffers.size(); i++) { delete [] _data->tileBuffers[i]->buffer; }

I think I understand. The delete expression in the catch block is deleting vr, but is qualified by something like vro->, and its vro that may not have been allocated due to the throw. In your example immediately above, vro might be _data.

Co-authored-by: Geoffrey White <40627776+geoffw0@users.noreply.github.com>

ihsinme · 2022-03-30T07:53:14Z

thanks for your fixes.
I'm a little late with the answer, I hope to answer all your questions soon

geoffw0 · 2022-04-04T18:37:52Z

Thanks for making the changes - we've definitely made the QL easier to follow!

I tried this query on LGTM but I didn't find any results (a few still to finish as I write this). It might be a good idea to start thinking of ways we could widen the query?

ihsinme · 2022-04-04T18:54:50Z

Thanks for making the changes - we've definitely made the QL easier to follow!

I tried this query on LGTM but I didn't find any results (a few still to finish as I write this). It might be a good idea to start thinking of ways we could widen the query?

my analysis showed several detections.
in projects using openexr for example.
https://github.com/mnaydenov/FreeImage-Adv.git

This project has already fixed a similar problem.
RTEMS/rtems-tools#2
https://github.com/RTEMS/rtems-tools.git

I am currently evaluating how to expand the detect if in the first part look for access without operator delete.

For the second part, I don't see extensions yet. I'll be glad for a hint.

geoffw0 · 2022-04-05T09:32:05Z

For the benefit of my future self and other readers, here's an LGTM run of the query on the two projects mentioned above: https://lgtm.com/query/410819352512416178/

ihsinme · 2022-05-04T15:13:59Z

good afternoon @geoffw0

Sorry for the delay in expanding this PR.
https://lgtm.com/query/4145429960886415578/

I suggest adding this version of detection, I like the preliminary results. although there are still a number of false positives associated with null initialization.

The idea behind the query is that an exception can be thrown when memory is allocated, and if it is freed during processing, then this behavior is undefined.

if you don't mind, I'll add that part to the main request.

geoffw0 · 2022-05-04T17:42:06Z

The new case seems to be finding good results, though the message will need to explain the problem clearly so that a user can make sense of what they're being shown (it needs to say that if the allocation in the try block fails, then an unallocated pointer will be freed in the catch block).

As an improvement, you should be able to spot variables that have been initialized to zero in a constructor initializer list using the ConstructorFieldInit class.

geoffw0

Great!

geoffw0 · 2022-05-05T15:11:33Z

It looks like there are some new results in the test, and the QL needs to be autoformatted.

ihsinme · 2022-05-05T16:18:51Z

@geoffw0
I made changes now you can run checks.

github-code-scanning

Found 4 vulnerabilities.

cpp/ql/src/experimental/Security/CWE/CWE-476/DangerousUseOfExceptionBlocks.ql

ihsinme added 3 commits Feb 25, 2022

Merge pull request #1 from github/main

3d1f4d5

up to head

Add files via upload

0257011

Add files via upload

a9a2ca3

ihsinme requested a review from as a code owner Feb 25, 2022

github-actions bot added C++ documentation labels Feb 25, 2022

MathiasVP assigned geoffw0 Feb 28, 2022

geoffw0 reviewed Mar 14, 2022

View changes

ihsinme and others added 2 commits Mar 15, 2022

Apply suggestions from code review

e99eaeb

Co-authored-by: Geoffrey White <40627776+geoffw0@users.noreply.github.com>

Update DangerousUseOfExceptionBlocks.cpp

62ecf54

ihsinme added 5 commits Mar 16, 2022

Update DangerousUseOfExceptionBlocks.ql

2959150

Update test.cpp

cd561dd

Update DangerousUseOfExceptionBlocks.expected

ccbb443

Update test.cpp

22cf3f7

Update DangerousUseOfExceptionBlocks.cpp

151c93f

geoffw0 reviewed Mar 23, 2022

View changes

Apply suggestions from code review

b950942

Co-authored-by: Geoffrey White <40627776+geoffw0@users.noreply.github.com>

ihsinme changed the title ~~en CPP: Add query for CWE-476: NULL Pointer Dereference when using exception handling blocks~~ CPP: Add query for CWE-476: NULL Pointer Dereference when using exception handling blocks Apr 2, 2022

Update DangerousUseOfExceptionBlocks.ql

61860c9

Update DangerousUseOfExceptionBlocks.ql

73de757

Update DangerousUseOfExceptionBlocks.expected

275b29a

Update DangerousUseOfExceptionBlocks.ql

75244ef

geoffw0 previously approved these changes May 5, 2022

View changes

Update DangerousUseOfExceptionBlocks.ql

2d4d7aa

ihsinme dismissed geoffw0’s stale review via 2d4d7aa May 5, 2022

ihsinme added 2 commits May 5, 2022

Update test.cpp

185a60f

Update DangerousUseOfExceptionBlocks.expected

6dec118

github-code-scanning bot found 4 vulnerabilities May 5, 2022

View changes

Update DangerousUseOfExceptionBlocks.ql

b98ddc7

github / codeql Public

CPP: Add query for CWE-476: NULL Pointer Dereference when using exception handling blocks #8245

CPP: Add query for CWE-476: NULL Pointer Dereference when using exception handling blocks #8245

ihsinme commented Feb 25, 2022

geoffw0 left a comment

ihsinme commented Mar 15, 2022

geoffw0 left a comment

geoffw0 Mar 23, 2022

ihsinme Apr 2, 2022

geoffw0 Apr 4, 2022

ihsinme commented Mar 30, 2022

geoffw0 commented Apr 4, 2022

ihsinme commented Apr 4, 2022

geoffw0 commented Apr 5, 2022

ihsinme commented May 4, 2022

geoffw0 commented May 4, 2022

geoffw0 left a comment

geoffw0 commented May 5, 2022

ihsinme commented May 5, 2022

github-code-scanning bot left a comment

github / codeql Public

CPP: Add query for CWE-476: NULL Pointer Dereference when using exception handling blocks #8245

Are you sure you want to change the base?

CPP: Add query for CWE-476: NULL Pointer Dereference when using exception handling blocks #8245

Conversation

ihsinme commented Feb 25, 2022

geoffw0 left a comment

ihsinme commented Mar 15, 2022

geoffw0 left a comment

geoffw0 Mar 23, 2022

Choose a reason for hiding this comment

ihsinme Apr 2, 2022

Choose a reason for hiding this comment

geoffw0 Apr 4, 2022

Choose a reason for hiding this comment

ihsinme commented Mar 30, 2022

geoffw0 commented Apr 4, 2022

ihsinme commented Apr 4, 2022

geoffw0 commented Apr 5, 2022

ihsinme commented May 4, 2022

geoffw0 commented May 4, 2022

geoffw0 left a comment

geoffw0 commented May 5, 2022

ihsinme commented May 5, 2022

github-code-scanning bot left a comment