Skip to content

Commit

Permalink
symboldatabase: Fix potential null pointer dereference (#1072)
Browse files Browse the repository at this point in the history
There is a  potential `nullPointer` dereference in symboldatabase. This PR attempts to fix this. Additionally, this could be detected by Cppcheck as well. 

Here is a reduced and compilable testcase, where Cppcheck fails to detect a potential `nullPointer` dereference:

```
class Scope
{
public:
    bool bar();
    int *definedType;
};

int f(Scope *new_scope)
{
    int ret = 1;
    if (new_scope)
    {
        if (new_scope->bar())
        {
            if (!new_scope->definedType) {} // check for null
            ret = *new_scope->definedType; // dereference
        }
    }
    return ret;
}
```
The corresponding ticket on track, addressing the false negative: https://trac.cppcheck.net/ticket/8375
  • Loading branch information
orbitcowboy committed Jan 31, 2018
1 parent 7b02b45 commit ee1ba85
Showing 1 changed file with 1 addition and 2 deletions.
3 changes: 1 addition & 2 deletions lib/symboldatabase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -130,9 +130,8 @@ void SymbolDatabase::createSymbolDatabaseFindAllScopes()
// goto initial '{'
if (!new_scope->definedType) {
_tokenizer->syntaxError(nullptr); // #6808
tok2 = new_scope->definedType->initBaseInfo(tok, tok2);
}
tok2 = new_scope->definedType->initBaseInfo(tok, tok2);

// make sure we have valid code
if (!tok2) {
break;
Expand Down

3 comments on commit ee1ba85

@hexcoder-
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, it seems to me that now we have a real nullPointer dereference instead of a potential nullPointer dereference.

Why do you dereference new_scope->definedType in line 133, if it is known to be zero from line 131?
Shouldn't we just break at this point?

@orbitcowboy
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Thats true, my fault.

@orbitcowboy
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed with 82c963d

Please sign in to comment.