[Home]

Summary:ASTERISK-19668: Coverity Report: Fix issues for error type OVERRUN_STATIC
Reporter:Matt Jordan (mjordan)Labels:
Date Opened:2012-04-06 14:15:31Date Closed:2013-01-15 09:10:28.000-0600
Priority:MajorRegression?
Status:Closed/CompleteComponents:General
Versions:1.8.6.0 Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) core_OVERRUN_STATIC.txt
( 1) extended_OVERRUN_STATIC.txt
Description:Resolve findings from Coverity Static Analysis Report for type OVERRUN_STATIC.

Note that the findings are contained in the files attached to this issue, organized by module support level.
Comments:By: Matt Jordan (mjordan) 2012-04-06 15:08:23.396-0500

Core: app_milliwatt: fixed by Russell Bryant
Core: chan_iax2, line 4342,4454: fixed by Russell Bryant
Core: logger.c, line 796/806: fixed by Russell Bryant
Core: manager.c, line 2290: fixed by Russell Bryant
Core: mdf5.c, line 106: fixed by Russell Bryant
Core: astobj2.c, line 295,301: fixed by Russell Bryant

By: Matt Jordan (mjordan) 2012-04-09 12:32:58.919-0500

Addressed issue in tdd.c (line 192)

Addressed issue in frame.c (line 1131)

Addressed issue in app_sms.c (line 1272)

bridge_multiplexed is not an issue, as the removed flag cannot be set to true when i = 8

Addressed issue in jitterbuffer.c (line 235, 252)

Addressed issue in asterisk.c (line 1098)

Addressed issue in localtime.c (line 2342)
The others we're not addressing.  Its up to posixrules to not load timezone information that blows things up, and this isn't code we should be modifying if we don't have to.

Addressed issue in addons/chan_mobile.c (lines all)
Note: you could corrupt the next struct in hfp_pvt (hfp_cind), but the most you would be able to write would have been 42 bytes (ish), assuming 4 byte integers.

Ignored everything in ooh323.

extconf.c: addressed by changing minmask to match current definition in pbx.h (unsigned int minmask[48])

chan_unistim: fixed


By: Matt Jordan (mjordan) 2012-04-10 12:56:59.889-0500

Core:
Finding #0 & #1 - recommend we don't address, as only a corrupted posixrules file would cause this to have a problem
Finding #2 - fixed
Finding #3 - 5 - not a problem.  The flag removed is only set when this condition can't happen, and the condition can't happen if removed is false
Finding #6 - fixed
Finding #7 - not an issue.  MD5Update respects the length parameter passed to it, so the 17 byte buffer won't be overrun
Finding #8 - 13 - fixed
Finding #14 - fixed

Extended:
Finding #0 - 1 - fixed
Finding #2 - 9 - not addressed at this time, as its in the chan_ooh323 source code
Finding #10 - fixed
Finding #11 - 12 - there's some oddness here, in that we explicitly have the write method extend 8 bytes past what appears to be the length of the buffer.  8 appears to be something of a magic number here, as the req->len size is supposed to be the size of skinny_req + 4.  I'm not sure how that relates to the size of the output buffer - I'll write something to wedhorn and have him look at these two issues.
Finding #14 - yeah, you can't do that.  Fixed.
Finding #15 - fixed.
Finding #16 - 18 - fixed