From: Christoph B. <bar...@gm...> - 2006-04-18 18:49:09
|
Hi, here is a report of items brought up by a static code checker. The tool is internal and not intended for the public. If you have questions about some items, please ask me directly. ------------------------------------------------------------------ Misc problems: ------------------------------------------------------------------ - src/libffmpeg/libavcodec/dv.c:888 pbs[6*5] is out of bounds. - src/libffmpeg/libavcodec/dv.c:873 If j reaches 29, then j+1 is out of bounds of pbs. - src/libffmpeg/libavcodec/asv1.c:293 When line 287 is true then ccp becomes 8 and the access is out of bounds. - src/libffmpeg/libavcodec/h261.c:267 If cbp is 0 as indicated by line 238, then the access is out of bounds. - src/libffmpeg/libavcodec/h263.c:1624 Index -1 is invalid. - src/libffmpeg/libavcodec/h264.c:7016 Is the ; intended? - src/libffmpeg/libavcodec/h264.c:6421 Out of bounds access for i bigger than 3. - src/libffmpeg/libavcodec/h264.c:7060 If aspect_ratio_idc is 14 or 15, you have an out of bounds access. - src/libffmpeg/libavcodec/ffv1.c:380 sample[2] is out of bounds. - src/libffmpeg/libavcodec/h264.c:4945 mv_cache[8] is out of bounds. - src/libffmpeg/libavcodec/imgconvert.c:1929 size is signed. - src/libffmpeg/libavcodec/mpegaudiodec.c:1507 If bits is 0 then line 1483 selects the else case and you get an invalid shift amount here. - src/libffmpeg/libavcodec/qdm2.c:1473,1476,1478 Variable i already controls the loop in line 1439. - src/libffmpeg/libavcodec/qdm2.c:541 If coding_method[ch][sb][j] - 8 is between 15 and 22 then coding_method[ch][sb][j] is between 23 and 30 and you access beyond array bounds here, because switchtable has only 23 entries. - src/libffmpeg/libavcodec/rangecoder.c:114 Off by one error, if i is 0. Then c->one_state[256-0] is beyond the array bounds. - src/libffmpeg/libavcodec/svq1.c:1020 Out of bound access. 1+count is always bigger than 1 the maximum allowed index. - src/libffmpeg/libavcodec/utils.c:881 && 0 is always false - src/libffmpeg/libavcodec/vp3.c:1305 fragment->next_coeff[coeff_index] is a Coeff and not an int - src/libffmpeg/libavcodec/ratecontrol.c:909 Using abs instead of fabs here will loose precision. - src/libffmpeg/libavcodec/ratecontrol.c:844 Only avg_quantizer[P_TYPE], avg_quantizer[I_TYPE] and avg_quantizer[B_TYPE] are initialized here. The other two are uninitialized. - src/libffmpeg/libavcodec/truemotion1.c:375 header.width, header.height, header.yoffset and header.xoffset are not initialized here. - src/libffmpeg/libavcodec/truemotion1.c:412 If header.compression is 17 then this line is an off by one error. Note that 17 is not excluded by the earlier check. - src/post/goom/gfontlib.c:12 The memory pointed to by gfont and hte allocations from line 38 till 44 are leaking here. - src/post/goom/tentacle3d.c:102 - src/post/goom/config_param.c:26 (similar) - src/post/goom/plugin_info.c:109 (similar) - src/post/goom/filters.c:747 (similar) - src/post/goom/ifs.c:769 (similar) Some of VisualFX values are not initialized. - src/xine-engine/osd.c:436 I guess the ; should not be after the if clause. - src/xine-engine/audio_out.c:1037 cur_time could be uninitialized here when the line is visited in the first loop. - src/xine-engine/vo_scale.c:261 If the if condition in line 250 is false, then a lot of values are not initialized here. - src/input/vcd/libcdio/cdio.c:926 If size is M2RAW_SECTOR_SIZE, then memcpy reads beyond the buf buffer that is only CDIO_CD_FRAMESIZE long. - src/input/libreal/asmrp.c:422 If i < 0 as indicated by line 419, then you have an out of bounds access in line 422. - src/input/libdvdnav/ifo_read.c:501,537,1405,1531 A freed pointer is exposed to the outside world. - src/libw32dll/wine/win32.c DWORD is unsigned and cannot be < 0. - src/libfaad/hcr.c:160 - src/input/libdvdnav/nav_read.c:68 (similar) If b->len is 32 then the shift amount is out of bounds. Allowed shift amounts are ranging from 0 to 31. - src/libfaad/mdct.c:199 - src/libfaad/mdct.c:205 (similar) src/libffmpeg/libavcodec/truemotion2.c:253 (similar) When k from the loop in line 193 reaches N8-1. Then N8-2-k is N8-2-N8+1 == -1. Then you are accessing array Z1 out of bounds. - src/libfaad/syntax.c:352 If channels+1 is equal to MAX_CHANNELS then the check in line 326 does not let the function exit. But in line 352 you have then an off by one error. - src/libxineadec/nosefart/fmopl.c:575 When i reaches the value 75 you write behind the array bounds of OPL->AR_TABLE. The last index is 74. - src/libfaad/syntax.c:2073 The loop is executed at most once. - src/libw32dll/wine/win32.c:1524 If index is 64 then one has an off by one error here. - src/libw32dll/wine/vfl.c:165,169,191,195 Calling abs instead of labs looses precision here. - src/input/libdvdnav/dvd_udf.c:603 Maybe cached_dir_base leaks here. ------------------------------------ Problems involving the NULL pointer: ------------------------------------ - src/libffmpeg/libavcodec/qdm2.c:1454 If line 1447 is never executed, then packet is NULL here. - src/xine-engine/video_overlay.c:280 If this->events[new_event].event is NULL as indicated by line 277, then line 280 crashes. - src/xine-engine/video_overlay.c:459 If this->events[this_event].event->object.overlay is NULL as indicated by line 429, then line 459 crashes. - src/xine-engine/audio_out.c:1128 If in_buf is NULL as indicated by line 1102, then line 1128 crashes. - src/libw32dll/wine/win32.c:3561 You call strcpy only when lpBuffer is NULL. - src/libw32dll/wine/win32.c:3135 If field is NULL as indicated by line 3129, then line 3135 calls strcpy with NULL as first argument. - src/libw32dll/DirectShow/inputpin.c:63 If pin->pin2 is NULL as indicated by line 45, then line 63 crashes. - src/libfaad/filtbank.c:128 - src/libfaad/filtbank.c:158 If no case is selected in the switch statement in line 140, then mdct is still NULL here. - src/libfaad/filtbank.c:276 - src/libfaad/filtbank.c:307 (similar) - src/libfaad/filtbank.c:395 (similar) window_short_prev is NULL here, if line 196 has not been executed. - misc/cdda_server.c:382 - src/input/libdvdnav/dvd_input.c:341 (similar) If the code from line 374-376 is executed then you should return from the function, because you can get a null pointer dereference in line 382 if dvdcss_version is NULL. ----------------------------------------------------------------- Cases from switch statements that fall through in some cases but do not have a fall through comment as in most such cases. ------------------------------------------------------------------ - src/libfaad/syntax.c:2078 - src/libfaad/syntax.c:2081 - src/libmpeg2new/libmpeg2/header.c:615 - src/libmpeg2new/libmpeg2/header.c:232 - src/input/libreal/rmff.c:476 - src/xine-engine/load_plugins.c:2512 - src/libffmpeg/libavcodec/utils.c:231 - src/libffmpeg/libavcodec/utils.c:226 - src/libffmpeg/libavcodec/rpza.c:159 - src/libffmpeg/libavcodec/indeo3.c:1002 - src/libffmpeg/libavcodec/indeo3.c:999 ----------------------------------------------------------------- Lines where boolean expressions are used in non-boolean contexts: I suspect that at least the lines marked with !!! are bugs ----------------------------------------------------------------- - src/libdts/parse.c:937 (! binds tigher than &) - src/libdts/parse.c:667 (can never be true) - src/input/libdvdnav/ifo_read.c:653,656 (always false) - src/libmpeg2new/libmpeg2/header.c:946 - src/libmpeg2new/libmpeg2/header.c:692 - src/post/audio/window.c:200 - src/post/goom/tentacle3d.c:246 - src/libffmpeg/libavutil/rational.c:36 |
From: Michael N. <mic...@gm...> - 2006-05-29 17:17:34
|
Hi non ffmpeg stuff snipped ... On Tue, Apr 18, 2006 at 08:49:05PM +0200, Christoph Bartoschek wrote: > Hi, > > here is a report of items brought up by a static code checker. The tool is > internal and not intended for the public. If you have questions about some > items, please ask me directly. > > ------------------------------------------------------------------ > Misc problems: > ------------------------------------------------------------------ > > - src/libffmpeg/libavcodec/dv.c:888 > > pbs[6*5] is out of bounds. > > - src/libffmpeg/libavcodec/dv.c:873 > > If j reaches 29, then j+1 is out of bounds of pbs. NULL is also out of bounds and its use wont crash your program its dereferencing which does and that doesnt happen here > > - src/libffmpeg/libavcodec/asv1.c:293 > > When line 287 is true then ccp becomes 8 and the access is out of > bounds. your checker has the same bug as coverity, this isnt possible > > - src/libffmpeg/libavcodec/h261.c:267 > > If cbp is 0 as indicated by line 238, then the access is out of bounds. not possible i think, added a assert() just to be sure > > - src/libffmpeg/libavcodec/h263.c:1624 > > Index -1 is invalid. maybe if you read the c standard very strictly, i dunno, but in practice this is correct and intended that way, feel free to send a patch if it bothers you > > - src/libffmpeg/libavcodec/h264.c:7016 > > Is the ; intended? probably not, fixed > > - src/libffmpeg/libavcodec/h264.c:6421 > > Out of bounds access for i bigger than 3. svn ffmpeg is not affected > > - src/libffmpeg/libavcodec/h264.c:7060 > > If aspect_ratio_idc is 14 or 15, you have an out of bounds access. svn ffmpeg is not affected > > - src/libffmpeg/libavcodec/ffv1.c:380 > > sample[2] is out of bounds. no i dont think so > > - src/libffmpeg/libavcodec/h264.c:4945 > > mv_cache[8] is out of bounds. i doubt that (assuming i found the corresponding line in ffmpeg svn) > > - src/libffmpeg/libavcodec/imgconvert.c:1929 > > size is signed. no clue what the problem is or which line that is in ffmpeg svn > > > - src/libffmpeg/libavcodec/mpegaudiodec.c:1507 > > If bits is 0 then line 1483 selects the else case and you get an invalid > shift amount here. not possible > > - src/libffmpeg/libavcodec/qdm2.c:1473,1476,1478 > > Variable i already controls the loop in line 1439. yep that doesnt look good, ill leave this and the other qdm2.c issues to the qdm2.c author/maintainer > > - src/libffmpeg/libavcodec/qdm2.c:541 > > If coding_method[ch][sb][j] - 8 is between 15 and 22 then > coding_method[ch][sb][j] is between 23 and 30 and you access beyond > array bounds here, because switchtable has only 23 entries. > - src/libffmpeg/libavcodec/rangecoder.c:114 > > Off by one error, if i is 0. Then c->one_state[256-0] is beyond the > array bounds. ffmpeg svn is not affected > > - src/libffmpeg/libavcodec/svq1.c:1020 > > Out of bound access. 1+count is always bigger than 1 the maximum > allowed index. disagree > > - src/libffmpeg/libavcodec/utils.c:881 > > && 0 is always false yes > > - src/libffmpeg/libavcodec/vp3.c:1305 > > fragment->next_coeff[coeff_index] is a Coeff and not an int this is just in a "debuging printf", ill leave this to the vp3 maintainer > > > - src/libffmpeg/libavcodec/ratecontrol.c:909 > > Using abs instead of fabs here will loose precision. fixed > > - src/libffmpeg/libavcodec/ratecontrol.c:844 > > Only avg_quantizer[P_TYPE], avg_quantizer[I_TYPE] and > avg_quantizer[B_TYPE] are initialized here. The other two are > uninitialized. IPB should be all types there are ... > > - src/libffmpeg/libavcodec/truemotion1.c:375 > > header.width, header.height, header.yoffset and header.xoffset are not > initialized here. > > - src/libffmpeg/libavcodec/truemotion1.c:412 > > If header.compression is 17 then this line is an off by one error. Note > that 17 is not excluded by the earlier check. ill leave these to the truemotion1 author/maintainer > [...] > ------------------------------------ > Problems involving the NULL pointer: > ------------------------------------ > > - src/libffmpeg/libavcodec/qdm2.c:1454 > > If line 1447 is never executed, then packet is NULL here. > [...] > ----------------------------------------------------------------- > Cases from switch statements that fall through in some cases but > do not have a fall through comment as in most such cases. > ------------------------------------------------------------------ sorry but wtf is a fall through comment? on which page of the c standard is that mentioned? > [...] > - src/libffmpeg/libavcodec/utils.c:231 > - src/libffmpeg/libavcodec/utils.c:226 > - src/libffmpeg/libavcodec/rpza.c:159 > - src/libffmpeg/libavcodec/indeo3.c:1002 > - src/libffmpeg/libavcodec/indeo3.c:999 > > ----------------------------------------------------------------- > Lines where boolean expressions are used in non-boolean contexts: > > I suspect that at least the lines marked with !!! are bugs > ----------------------------------------------------------------- > [...] > - src/libffmpeg/libavutil/rational.c:36 another random piece of correct code ... [...] -- Michael In the past you could go to a library and read, borrow or copy any book Today you'd get arrested for mere telling someone where the library is |
From: Darren S. <li...@yo...> - 2006-05-29 17:43:43
|
I demand that Michael Niedermayer may or may not have written... > On Tue, Apr 18, 2006 at 08:49:05PM +0200, Christoph Bartoschek wrote: [snip] >> - src/libffmpeg/libavcodec/imgconvert.c:1929 >> size is signed. > no clue what the problem is or which line that is in ffmpeg svn "if (size < 0)" in avpicture_alloc(). The problem is that size is declared as unsigned int. [snip] >> ----------------------------------------------------------------- >> Cases from switch statements that fall through in some cases but >> do not have a fall through comment as in most such cases. >> ------------------------------------------------------------------ > sorry but wtf is a fall through comment? [...] It's much like a fall through air, but you probably won't land on the ground. ... oh, you meant "fall-through comment"... ;-) A comment placed to indicate the deliberate omission of 'break;', 'continue;' etc. before a 'case' or 'default' label in a switch block. [snip] -- | Darren Salt | linux or ds at | nr. Ashington, | Toon | RISC OS, Linux | youmustbejoking,demon,co,uk | Northumberland | Army | + Generate power using sun, wind, water, nuclear. FORGET COAL AND OIL. You will never know hunger. |