Opened 5 years ago

Closed 3 years ago

#3430 closed defect (complete)

illegal memory access - coverity

Reported by: wlodekwencel Owned by: tomek
Priority: low Milestone: Outstanding Tasks
Component: dhcp4 Version: git
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: DHCP Feature Depending on Ticket:
Estimated Difficulty: 0 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description (last modified by tomek)

Issues reported by Coverity scan performed Apr 15 on code from master.

bugs no: 1202665 and 1202663

illegal access to memory in file /scr/lib/dhcp/pkt4.cc

https://scan5.coverity.com:8443/reports.htm#v35004/p10289

Subtickets

Change History (18)

comment:1 Changed 5 years ago by wlodekwencel

  • Component changed from Unclassified to dhcp4

comment:2 Changed 5 years ago by wlodekwencel

  • Milestone changed from New Tasks to Kea-proposed

comment:3 Changed 5 years ago by tomek

  • Milestone changed from Kea-proposed to Kea0.9

comment:4 Changed 5 years ago by stephen

More information can be found in #3442 (now closed as it is a duplicate of this ticket).

comment:5 Changed 5 years ago by tomek

  • Priority changed from medium to low

comment:6 Changed 5 years ago by stephen

  • Owner set to stephen
  • Status changed from new to accepted

comment:7 Changed 5 years ago by stephen

  • Owner changed from stephen to UnAssigned
  • Status changed from accepted to reviewing

The changes made are:

  • Change the construct &array[n] to array + n in a bid to avoid Coverity's warning about the end iterator in an STL construct pointing to an invalid element. (At some point in the future we should move to C++11, at which point the STL algorithms fill_n and copy_n become available, which deal with this once and for all.)
  • Updated the unit tests. The main reason for this was that as it stood, the test data created for the test with sname/file length of 1 was the same as that for the test with a length of 0.

No ChangeLong entry is envisaged, as this change is not visible to the user.

comment:8 Changed 5 years ago by tmark

  • Owner changed from UnAssigned to tmark

comment:9 Changed 5 years ago by tmark

  • Owner changed from tmark to stephen

Changes look fine and pass unit tests under OS-X. It will interesting to see if this does indeed satisfy Coverity's complaint.

Please merge your changes.

comment:10 Changed 5 years ago by stephen

  • Resolution set to fixed
  • Status changed from reviewing to closed

Merged in commit ccfd61b4b908254dcd968d26a2d39f17971146ed

comment:11 Changed 5 years ago by sar

  • Resolution fixed deleted
  • Status changed from closed to reopened
  • Version set to git

The change didn't satisfy Coverity's complaint.

I'm not sure if the proper procedure is to re-open the ticket or to create a new one. I'll re-openit and we can close it and open another if necessary.

comment:12 Changed 5 years ago by hschempf

  • Milestone changed from Kea0.9 to DHCP Outstanding Tasks

Consensus at Kea team meeting 25 Feb - move this ticket from 0.9 to DHCP outstanding. Coverity flags issue as 'high' but team will not fix in 0.9.1 final.

comment:13 Changed 5 years ago by fdupont

Two ideas:

  • use std::memset and std::memcpy
  • use an explicit loop with a test for copy/clear

As far as I can understand methods are setName and setFile.
BTW is it possible to get the complaint in the ticket?

Last edited 5 years ago by fdupont (previous) (diff)

comment:14 Changed 5 years ago by fdupont

Ping?

comment:15 Changed 4 years ago by fdupont

Is this ticket still active? (i.e., does Coverity still complain?)

comment:16 Changed 4 years ago by tomek

  • Milestone changed from DHCP Outstanding Tasks to Outstanding Tasks

Milestone renamed

comment:17 Changed 3 years ago by tomek

  • Description modified (diff)
  • Owner changed from stephen to tomek
  • Status changed from reopened to assigned

I just checked. Coverity doesn't complain about it. We can close this ticket.

Last edited 3 years ago by tomek (previous) (diff)

comment:18 Changed 3 years ago by tomek

  • Resolution set to complete
  • Status changed from assigned to closed
Note: See TracTickets for help on using tickets.