Page MenuHomePhabricator

[analyzer] CERT STR rule checkers: STR50-CPP
Needs RevisionPublic

Authored by Charusso on May 24 2020, 9:45 PM.



This patch introduces a new experimental checker:

This checker is implemented based on the following rule:

It warns on misusing std::cin and std::basic_istream<T>::read() to
create a std::string which may not null-terminated.

Diff Detail

Event Timeline

Charusso created this revision.May 24 2020, 9:45 PM
baloghadamsoftware requested changes to this revision.Jun 26 2020, 6:09 AM

Why limiting the checker to std::cin?

Anyway, this checker is far from ready yet (the essence is missing), see my comment inside.


Hmm, let us imagine that I write the following code:

TypeT cin;
AnyClassWithInstanceGTGTOperatorTakingTypeT >> cin;

Your code will probably crash when trying to retrieve arg 1 because you neither check for std::cin nor for InstanceCall.


Do we need such heavyweight thing here? In my experience, AST matchers are expensive to create.


Here should be the check for the size of the buffer and the comparison to cin.width(). It seems that it is not ready yet. This should be the most essential part of this checker.


std::cin does not write. It is just a stream from where we read. operator>> writes. Please rephrase the error message.


We should get warning for bufTwo and no warning for bufOne. operator>> reads cin.width()-1 characters to the buffer plus a null-terminator, which is exactly 12 bytes. So no overflow happens. However, it also resets the width, so the next read is unlimited. Please check STR50-CPP. It describes exactly the same as above.

This revision now requires changes to proceed.Jun 26 2020, 6:09 AM
whisperity added inline comments.Jun 26 2020, 6:25 AM

IIRC the matches are of SmallVector which should have an empty() method, prefer it to size() <op> 0.