Skip to content

parser-cov: align assignment of key events with Coverity #165

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Feb 26, 2024
Merged

parser-cov: align assignment of key events with Coverity #165

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
250e301
parser-json: remove trailing new-lines from messages
kdudka Feb 20, 2024
5b25c7d
parser-cov: align the columns in KeyEventDigger ctor
kdudka Feb 20, 2024
0a0c57e
parser-cov: tweak key event detection for LOCK_EVASION
kdudka Feb 19, 2024
5f3f915
parser-cov: update explicitly defined key events
kdudka Feb 20, 2024
6dffa0d
parser-cov: look for an explicitly defined key event
kdudka Feb 20, 2024
737f94a
parser-cov: look for an eligible key event
kdudka Feb 20, 2024
c9d76f2
parser-cov: update explicitly defined key events
kdudka Feb 21, 2024
21fd2f7
parser-cov: search key event backwards
kdudka Feb 21, 2024
9dbb09a
parser-cov: search more key event backwards
kdudka Feb 21, 2024
75a2a82
parser-cov: further tweaks to key event matching
kdudka Feb 21, 2024
4a17373
tests/csdiff: help csdiff to find the correct key event
kdudka Feb 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
185 changes: 125 additions & 60 deletions src/lib/parser-cov.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ struct KeyEventDigger::Private {
typedef std::set<std::string> TSet;
typedef std::map<std::string, TSet> TMap;
TMap hMap;
TSet denyList, traceEvts;
TSet denyList, traceEvts, searchBackwards;
const RE reEvtSuffix = RE("^(.*)\\[[^ \\]]+\\]$");
const std::string stripEvtName(const std::string &) const;
};
Expand All @@ -227,44 +227,69 @@ KeyEventDigger::KeyEventDigger():
d(new Private)
{
// register checker-specific key events
d->hMap["BAD_CHECK_OF_WAIT_COND"].insert("wait_cond_improperly_checked");
d->hMap["BAD_LOCK_OBJECT"] .insert("boxed_lock");
d->hMap["BAD_LOCK_OBJECT"] .insert("lock_on_assigned_field");
d->hMap["BAD_LOCK_OBJECT"] .insert("single_thread_lock");
d->hMap["CALL_SUPER"] .insert("missing_super_call");
d->hMap["CHECKED_RETURN"] .insert("check_return");
d->hMap["CONSTANT_EXPRESSION_RESULT"].insert("extra_high_bits");
d->hMap["CONSTANT_EXPRESSION_RESULT"].insert("logical_vs_bitwise");
d->hMap["CONSTANT_EXPRESSION_RESULT"].insert("missing_parentheses");
d->hMap["CONSTANT_EXPRESSION_RESULT"].insert("operator_confusion");
d->hMap["CONSTANT_EXPRESSION_RESULT"].insert("pointless_expression");
d->hMap["CONSTANT_EXPRESSION_RESULT"].insert("result_independent_of_operands");
d->hMap["CONSTANT_EXPRESSION_RESULT"].insert("same_on_both_sides");
d->hMap["EXPLICIT_THIS_EXPECTED"].insert("implicit_this_used");
d->hMap["FORWARD_NULL"] .insert("deref_parm");
d->hMap["FORWARD_NULL"] .insert("dereference");
d->hMap["FORWARD_NULL"] .insert("var_deref_op");
d->hMap["FORWARD_NULL"] .insert("var_deref_model");
d->hMap["LOCK"] .insert("double_lock");
d->hMap["LOCK"] .insert("double_unlock");
d->hMap["MISSING_BREAK"] .insert("unterminated_case");
d->hMap["NESTING_INDENT_MISMATCH"].insert("dangling_else");
d->hMap["NESTING_INDENT_MISMATCH"].insert("multi_stmt_macro");
d->hMap["NESTING_INDENT_MISMATCH"].insert("on_same_line");
d->hMap["NESTING_INDENT_MISMATCH"].insert("uncle");
d->hMap["ORDER_REVERSAL"] .insert("lock_acquire");
d->hMap["OVERRUN_STATIC"] .insert("index_parm");
d->hMap["OVERRUN_STATIC"] .insert("overrun-buffer-arg");
d->hMap["OVERRUN_STATIC"] .insert("overrun-local");
d->hMap["STREAM_FORMAT_STATE"] .insert("format_changed");
d->hMap["UNINIT"] .insert("uninit_use");
d->hMap["UNINIT"] .insert("uninit_use_in_call");
d->hMap["UNINIT_CTOR"] .insert("uninit_member");
d->hMap["USE_AFTER_FREE"] .insert("deref_after_free");
d->hMap["USE_AFTER_FREE"] .insert("deref_arg");
d->hMap["USE_AFTER_FREE"] .insert("double_free");
d->hMap["USE_AFTER_FREE"] .insert("pass_freed_arg");
d->hMap["USE_AFTER_FREE"] .insert("use_after_free");
d->hMap["ALLOC_FREE_MISMATCH"] .insert("free");
d->hMap["ARRAY_VS_SINGLETON"] .insert("callee_ptr_arith");
d->hMap["ARRAY_VS_SINGLETON"] .insert("ptr_arith");
d->hMap["ATOMICITY"] .insert("use");
d->hMap["BAD_CHECK_OF_WAIT_COND"] .insert("wait_cond_improperly_checked");
d->hMap["BAD_FREE"] .insert("incorrect_free");
d->hMap["BAD_LOCK_OBJECT"] .insert("boxed_lock");
d->hMap["BAD_LOCK_OBJECT"] .insert("lock_on_assigned_field");
d->hMap["BAD_LOCK_OBJECT"] .insert("single_thread_lock");
d->hMap["BAD_SHIFT"] .insert("large_shift");
d->hMap["BAD_SHIFT"] .insert("negative_shift");
d->hMap["CALL_SUPER"] .insert("missing_super_call");
d->hMap["CHECKED_RETURN"] .insert("check_return");
d->hMap["CHROOT"] .insert("chroot_call");
d->hMap["COM.BAD_FREE"] .insert("free");
d->hMap["CTOR_DTOR_LEAK"] .insert("alloc_fn");
d->hMap["CTOR_DTOR_LEAK"] .insert("alloc_new");
d->hMap["DEADCODE"] .insert("dead_error_begin");
d->hMap["DEADCODE"] .insert("dead_error_line");
d->hMap["EXPLICIT_THIS_EXPECTED"] .insert("implicit_this_used");
d->hMap["LOCK"] .insert("double_lock");
d->hMap["LOCK"] .insert("double_unlock");
d->hMap["LOCK"] .insert("missing_unlock");
d->hMap["LOCK_EVASION"] .insert("thread1_overwrites_value_in_field");
d->hMap["LOCK_EVASION"] .insert("thread2_checks_field_early");
d->hMap["LOCK_INVERSION"] .insert("lock_order");
d->hMap["INFINITE_LOOP"] .insert("loop_top");
d->hMap["MISSING_BREAK"] .insert("unterminated_case");
d->hMap["MISSING_RESTORE"] .insert("end_of_path");
d->hMap["MISSING_RESTORE"] .insert("end_of_scope");
d->hMap["MISSING_RESTORE"] .insert("exception");
d->hMap["MULTIPLE_INIT_SMART_PTRS"] .insert("multiple_init_smart_ptr");
d->hMap["NESTING_INDENT_MISMATCH"] .insert("actual_if");
d->hMap["NESTING_INDENT_MISMATCH"] .insert("multi_stmt_macro");
d->hMap["NESTING_INDENT_MISMATCH"] .insert("on_same_line");
d->hMap["NESTING_INDENT_MISMATCH"] .insert("uncle");
d->hMap["OPEN_REDIRECT"] .insert("sink");
d->hMap["ORDER_REVERSAL"] .insert("lock_order");
d->hMap["OS_CMD_INJECTION"] .insert("os_cmd_sink");
d->hMap["OVERLAPPING_COPY"] .insert("overlapping_assignment");
d->hMap["OVERLAPPING_COPY"] .insert("overlapping_copy");
d->hMap["OVERRUN_STATIC"] .insert("index_parm");
d->hMap["OVERRUN_STATIC"] .insert("overrun-buffer-arg");
d->hMap["OVERRUN_STATIC"] .insert("overrun-local");
d->hMap["RESOURCE_LEAK"] .insert("leaked_handle");
d->hMap["RESOURCE_LEAK"] .insert("leaked_storage");
d->hMap["RESOURCE_LEAK"] .insert("overwrite_var");
d->hMap["REVERSE_INULL"] .insert("check_after_deref");
d->hMap["REVERSE_NEGATIVE"] .insert("check_after_sink");
d->hMap["SENSITIVE_DATA_LEAK"] .insert("sink");
d->hMap["SERVLET_ATOMICITY"] .insert("set_attribute");
d->hMap["STREAM_FORMAT_STATE"] .insert("end_of_path");
d->hMap["TAINTED_SCALAR"] .insert("tainted_data");
d->hMap["TOCTOU"] .insert("fs_check_call");
d->hMap["UNEXPECTED_CONTROL_FLOW"] .insert("continue_in_do_while_false");
d->hMap["UNINIT"] .insert("uninit_use");
d->hMap["UNINIT"] .insert("uninit_use_in_call");
d->hMap["UNINIT_CTOR"] .insert("member_not_init_in_gen_ctor");
d->hMap["UNINIT_CTOR"] .insert("uninit_member");
d->hMap["UNLOCKED_ACCESS"] .insert("thread_unsafe_modification");
d->hMap["VARARGS"] .insert("missing_va_end");
d->hMap["WRAPPER_ESCAPE"] .insert("escape");
d->hMap["WRAPPER_ESCAPE"] .insert("use_after_free");

// we use COMPILER_WARNING as checker for compiler errors/warnings
d->hMap["COMPILER_WARNING"] .insert("error");
Expand All @@ -282,6 +307,29 @@ KeyEventDigger::KeyEventDigger():
// OWASP ZAP uses "alert" as the key event
d->hMap["OWASP_ZAP_WARNING"] .insert("alert");

// list of checkers where we take the _last_ matched key event
d->searchBackwards.insert("COMPILER_WARNING");
d->searchBackwards.insert("CONSTANT_EXPRESSION_RESULT");
d->searchBackwards.insert("DELETE_ARRAY");
d->searchBackwards.insert("FORWARD_NULL");
d->searchBackwards.insert("HARDCODED_CREDENTIALS");
d->searchBackwards.insert("HEADER_INJECTION");
d->searchBackwards.insert("INSUFFICIENT_LOGGING");
d->searchBackwards.insert("LOCK");
d->searchBackwards.insert("INVALIDATE_ITERATOR");
d->searchBackwards.insert("NULL_RETURNS");
d->searchBackwards.insert("OVERRUN");
d->searchBackwards.insert("PATH_MANIPULATION");
d->searchBackwards.insert("RESOURCE_LEAK");
d->searchBackwards.insert("RETURN_LOCAL");
d->searchBackwards.insert("UNINIT");
d->searchBackwards.insert("UNINIT_CTOR");
d->searchBackwards.insert("UNUSED_VALUE");
d->searchBackwards.insert("URL_MANIPULATION");
d->searchBackwards.insert("USE_AFTER_FREE");
d->searchBackwards.insert("VOLATILE_ATOMICITY");
d->searchBackwards.insert("WRITE_CONST_FIELD");

// events that should never be used as key events (excluding trace events)
d->denyList.insert("another_instance");
d->denyList.insert("comparison_remediation");
Expand Down Expand Up @@ -352,42 +400,59 @@ bool KeyEventDigger::guessKeyEvent(Defect *def)
// use the corresponding set of events from d->hMap
pKeyEvents = &it->second;

for (int idx = evtCount - 1U; idx >= 0; --idx) {
// look for an explicitly defined key event
bool found = false;
for (unsigned idx = 0U; idx < evtCount; ++idx) {
const DefEvent &evt = evtList[idx];
const std::string evtName = d->stripEvtName(evt.event);
if (!pKeyEvents->count(evtName))
continue;

// matched
def->keyEventIdx = idx;
return true;
found = true;
if (!d->searchBackwards.count(def->checker))
// checker not listed in d->searchBackwards --> take the first match
break;
}

// use the last event as key event by default (unless deny-listed)
for (int pass = 0; pass < 2; ++pass) {
for (int idx = evtCount - 1U; idx >= 0; --idx) {
const DefEvent &evt = evtList[idx];
if (evt.event == "#")
// never use comment as the key event
continue;

if (found)
return true;

// never use trace or deny-listed event as the key event
// (but pick the last one of there are no other events)
if (!pass) {
const std::string &evtName = evt.event;
if (d->traceEvts.count(evtName) || d->denyList.count(evtName))
continue;
}
// take the first eligible key event
bool valid = false;
bool eligible = false;
for (unsigned idx = 0; idx < evtCount; ++idx) {
const DefEvent &evt = evtList[idx];
if (evt.event == "#")
// never use comment as the key event
continue;

// matched
if (!valid) {
// at least we found an event which is not a comment
def->keyEventIdx = idx;
return true;
valid = true;
}

const bool findLastMatch = d->searchBackwards.count(def->checker);
if (findLastMatch && !eligible)
// no eligible event yet --> select the _last_ valid event
def->keyEventIdx = idx;

// skip trace and deny-listed events
const std::string &evtName = evt.event;
if (d->traceEvts.count(evtName) || d->denyList.count(evtName))
continue;

// matched
def->keyEventIdx = idx;
eligible = true;
if (!findLastMatch)
// checker not listed in d->searchBackwards --> take the first match
break;
}

// no valid key event
return false;
return valid;
}

void KeyEventDigger::initVerbosity(Defect *def)
Expand Down
15 changes: 15 additions & 0 deletions src/lib/parser-json.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,20 @@
#include "parser-json-simple.hh"
#include "parser-json-zap.hh"

#include <boost/algorithm/string/trim.hpp>
#include <boost/property_tree/json_parser.hpp>

void removeTrailingNewLines(Defect *pDef)
{
const auto isNewLine = [](const char c){ return c == '\n'; };

// go through all events and remove trailing new-lines from messages
// because they cannot be propagated through the plain-text format
// (and could cause artificial differences when compared with JSON)
for (DefEvent &evt : pDef->events)
boost::trim_right_if(evt.msg, isNewLine);
}

struct JsonParser::Private {
using TDecoderPtr = std::unique_ptr<AbstractTreeDecoder>;

Expand Down Expand Up @@ -136,6 +148,9 @@ bool JsonParser::getNext(Defect *def)
if (ok)
d->defNumber++;

// remove trailing new-lines from messages
removeTrailingNewLines(def);

return ok;
}
catch (pt::ptree_error &e) {
Expand Down
4 changes: 4 additions & 0 deletions tests/csdiff/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,5 +80,9 @@ test_csdiff(diff-misc 12-shellcheck-sc222x)
test_csdiff(diff-misc 13-gcca-filt)
test_csdiff(diff-misc 14-gitleaks-paths)
test_csdiff(diff-misc 15-gcc-prof-filter)
test_csdiff(diff-misc 16-cov-parser-key-event)
test_csdiff(diff-misc 17-cov-parser-key-event)
test_csdiff(diff-misc 18-cov-parser-key-event)
test_csdiff(diff-misc 19-cov-parser-key-event)

add_subdirectory(filter-file)
Empty file added tests/csdiff/diff-misc/16-cov-parser-key-event-add-z.err
View file
Open in desktop
Empty file.
Empty file added tests/csdiff/diff-misc/16-cov-parser-key-event-add.err
View file
Open in desktop
Empty file.
Empty file added tests/csdiff/diff-misc/16-cov-parser-key-event-fix-z.err
View file
Open in desktop
Empty file.
Empty file added tests/csdiff/diff-misc/16-cov-parser-key-event-fix.err
View file
Open in desktop
Empty file.
Loading