Skip to content

add CERT STR03-C check#1898

Merged
danmar merged 4 commits into
cppcheck-opensource:masterfrom
fuzzelhjb:CERT-STR03-C
Jun 24, 2019
Merged

add CERT STR03-C check#1898
danmar merged 4 commits into
cppcheck-opensource:masterfrom
fuzzelhjb:CERT-STR03-C

Conversation

@fuzzelhjb
Copy link
Copy Markdown
Contributor

No description provided.

Comment thread addons/cert.py Outdated
# Do not inadvertently truncate a string
def str03(data):
for token in data.tokenlist:
if simpleMatch(token, 'strncpy ('): # now search if the 3rd parameter is a sizeof()
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I would suggest a early continue here.

if not simpleMatch(token, 'strncpy ('):
    continue

Comment thread addons/cert.py Outdated
for token in data.tokenlist:
if simpleMatch(token, 'strncpy ('): # now search if the 3rd parameter is a sizeof()
paramToken = token.astParent
if paramToken is None:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I suggest a more strict condition if paramToken != token.next

Comment thread addons/cert.py Outdated
paramToken = token.astParent
if paramToken is None:
continue
if not paramToken.astOperand2 is None:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

hmm.. this looks clumpsy.. maybe we should create a utility function in cppcheckdata to get the parameters in a function call. We have such function in cppcheck core and its name is getArguments

Comment thread addons/cert.py Outdated
if lengthOp2 is None:
continue
if simpleMatch(lengthOp2.astOperand1, 'sizeof ('):
reportError(token, 'style', 'Do not inadvertently truncate a string', 'STR03-C')
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

hmm.. isn't it typical to use a sizeof in the 3rd argument when using strncpy. I fear that this will warn about typical strncpy usage.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We do allow more noise from addons. But I am uncertain about how much we can allow.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Well the check fullfills rule CERT-STR03-C as defined here: https://wiki.sei.cmu.edu/confluence/display/c/STR03-C.+Do+not+inadvertently+truncate+a+string
The reason for this check is that if you use strncpy with a sizeof() as length it could happen that there isn't a null termination in the first n characters you want to copy. This would lead to a non null-terminated string but strncpy/strcpy normaly indicates that the string is automatically null-terminated ;)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I have a bad feeling about it but I guess we should follow the cert guidelines.

fuzzelhjb and others added 3 commits June 19, 2019 12:22
…-STR03-C

# Conflicts:
#	addons/cert.py
#	addons/test/cert-test.c
@danmar danmar merged commit f36d671 into cppcheck-opensource:master Jun 24, 2019
jubnzv pushed a commit to jubnzv/cppcheck that referenced this pull request Nov 13, 2019
* add CERT STR03-C check

* fix cert test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants