on reports from Synopsys Coverity, Defensics and Protecode
Dan Poirot
dtpoirot at gmail.com
Fri Apr 1 02:07:30 UTC 2016
Hi Mark,
Happy to help. (I need to remember to kill the MS Outlook formatting...)
'Automatic bug filing' is a feature provided in response to 'aggressive' customer demand. ...they commonly set it up, try it out, hate it and then they promptly disable it. It can be done from the command line and may be limited to a single checker or a group of checkers. The use case most commonly seen is "I absolutely need to know if I see issues in this specific checker!"
It is starting to get some traction now that more developers are sensitive to (and potentially liable for?) any identifiable occurrence of PCI-DSS defects, OWASP Top 10 and CWE 25.
With regards to auto-assignment, we don't just use the basic 'git blame'.
We have a number of scenarios including the following:
SCM rule SCOPE
Set to last user to modify file of main event file
Set to last user to modify line of main event line
Set to last user to modify the function of main event function
Set to last user to modify top of the event tree top_events
Set to last user to modify any line in the event tree all_events
Set to last user to modify any function in the event tree all_functions
Set to last user to modify any file in the event tree all_files
Additionally, the project may be divided up using a regex of the file system into 'Components'. New issues may be assigned to the owner of the Component - typically a team lead.
A common Component-based work flow is to have new issues sent to a sub-project lead who then dispositions them to his or her team. Issue assignment is changeable over time based on a user's RBAC permissions.
All that said, using a conservative set of switches, I find a total of 37 issues with 25 having already been 'dismissed' by Eric as intentional, noisy or potentially false positive.
I only included auto-assignment for the seven of you who are listed as 'Members' on the GitLab roster.
Of those, Eric gets four - all of which are 'Low' impact and likely (and rightfully) to be ignored forever. Amar gets four 'Low' in the test code - all of which are parse warnings regarding escaped strings which could potentially not be portable (suppressible).
None of you were identified in the event tree of the 'Final Four'. Three are 'Low' impact and also uninteresting.
...the last one is pretty cool! ;-)
The function ntpdig_addremove_fd() in main.c of ntpdig has two occurrences of 'dereference after null check' (FORWARD_NULL) of the function resource 'c' - line 1081 and line 1096:
1054void ntpdig_addremove_fd(
1055 int fd,
1056 int is_pipe,
1057 int remove_it
1058 )
1059{
1060 u_int idx;
1061 blocking_child *c;
1062 struct event * ev;
1063
1. Condition is_pipe, taking false branch
1064 if (is_pipe) {
1065 msyslog(LOG_ERR, "fatal: pipes not supported on systems with socketpair()");
1066 exit(1);
1067 }
1068
1069 c = NULL;
2. Condition idx < blocking_children_alloc, taking true branch
5. Condition idx < blocking_children_alloc, taking true branch
9. Condition idx < blocking_children_alloc, taking false branch
1070 for (idx = 0; idx < blocking_children_alloc; idx++) {
1071 c = blocking_children[idx];
3. Condition NULL == c, taking true branch
6. Condition NULL == c, taking true branch
7. var_compare_op: Comparing c to null implies that c might be null.
1072 if (NULL == c)
4. Continuing loop
8. Continuing loop
1073 continue;
1074 if (fd == c->resp_read_pipe)
1075 break;
1076 }
10. Condition idx == blocking_children_alloc, taking false branch
1077 if (idx == blocking_children_alloc)
1078 return;
1079
11. Condition remove_it, taking false branch
1080 if (remove_it) {
CID 10009 (#1 of 2): Dereference after null check (FORWARD_NULL) [select issue]
1081 ev = c->resp_read_ctx;
1082 c->resp_read_ctx = NULL;
1083 event_del(ev);
1084 event_free(ev);
1085
1086 return;
1087 }
1088
1089 ev = event_new(base, fd, EV_READ | EV_PERSIST,
1090 &worker_resp_cb, c);
12. Condition NULL == ev, taking false branch
1091 if (NULL == ev) {
1092 msyslog(LOG_ERR,
1093 "ntpdig_addremove_fd: event_new(base, fd) failed!");
1094 return;
1095 }
CID 10009 (#2 of 2): Dereference after null check (FORWARD_NULL)13. var_deref_op: Dereferencing null pointer c.
1096 c->resp_read_ctx = ev;
1097 event_add(ev, NULL);
1098}
From: Mark Atwood [mailto:fallenpegasus at gmail.com]
Sent: Thursday, March 31, 2016 6:36 PM
To: Daniel Poirot <dtpoirot at gmail.com>; devel at ntpsec.org
Subject: on reports from Synopsys Coverity, Defensics and Protecode
Hi!
Thank you for setting up the Synopsys suite for NTPsec.
I've almost never seen automatic bugfiling by a robot work well.
Modulo that, automatically assigning to the developer that "owns" that line of code is not something we want to do on NTPsec, for two reasons, one philosophical, and one practical. First, we want to discourage "code ownership" by individual developers. Two, when you run "git blame" against an arbitrary line of code in NTPsec, it will probably name someone who is not part of our current team.
If the report is readable in a textmode email reader client, and if the report can be configured to just show new issues, emailing it to mailto:devel at ntpsec.org would be effective. On what schedule is an interesting question.
..m
On Wed, Mar 30, 2016 at 4:19 PM Daniel Poirot <mailto:dtpoirot at gmail.com> wrote:
I am starting up a NTPsec instance of the suite of Synopsys development testing tools - Coverity, Defensics and Protecode.
One feature of the static analysis tool is the assignment of issue by user name associated with the line of code in the source code management tool.
Triaged issues can be manually or automatically exported into a bug tracking system for further disposition.
Additionally, notification of newly detected issues can be emailed to the group, lead or SCM submitter.
Is there any interest in this level of integration and reporting?
Best regards,
Dan
_______________________________________________
devel mailing list
mailto:devel at ntpsec.org
http://lists.ntpsec.org/mailman/listinfo/devel
More information about the devel
mailing list