wine 0.9.39: Calysto v1.4 NULL dereference reports

[Domagoj Babic]

Hello,

I finally found time to go through Calysto reports for wine. Here are
the warnings related to applications. For libraries, since Calysto
doesn't know the calling context, any parameter ptr dereference without
checking is considered a bug. We can go over those warnings later if
anyone is really interested.

Here we go:

---------------------------------------------------------------
wine 0.9.39:

Calysto: ??/11

[Please replace ?? with the number of reports that require some
corrective action, adding assertion is considered a corrective action.
For every report, leave + if requires corrective action, replace with -
if it's definitely false positive, and replace with ? if unknown.
For false positives, please provide explanations. For ?, please ask what's
unclear. If you discover new bugs that Calysto should have caught,
please mention that as well.]

*** Analyzing /work/benchmarks/BINARY/wine_v0.9.39/tools_fnt2bdf.bc ***

Possible NULL-ptr deref (vc32):
@/work/benchmarks/SOURCES/wine-0.9.39/tools/fnt2bdf.c:91
+ main (fnt2bdf.c:548) calls return_data_value (@572), passing it a
pointer to &(pTInfo->type_id), which return_data_value dereferences (@91).
pTInfo aliases (lpdata+2), and lpdata was initialized with a call to
get_resource_table (@561), which can set it to NULL (either if malloc
(@534) returns NULL, or if it is set explicitly to NULL (@540)).

*** Analyzing /work/benchmarks/BINARY/wine_v0.9.39/tools_makedep.bc ***

Possible NULL-ptr deref (vc796):
@/work/benchmarks/SOURCES/wine-0.9.39/tools/makedep.c:196
+ Ptr p xmalloc-ed (makedep.c:187) dereferenced. Note that although
xmalloc checks pointer (util.c:31), report(R_FATAL, "msg") does not
exit, but only reports the msg and returns, so p (NULL) will be
still dereferenced.

*** Analyzing /work/benchmarks/BINARY/wine_v0.9.39/tools_sfnt2fnt.bc ***

Possible NULL-ptr deref (vc47):
@/work/benchmarks/SOURCES/wine-0.9.39/tools/sfnt2fnt.c:240
+ malloc-ed ptr eblc dereferenced without checking

Possible NULL-ptr deref (vc58):
@/work/benchmarks/SOURCES/wine-0.9.39/tools/sfnt2fnt.c:321
+ malloc-ed ptr dfCharTable dereferenced without checking

Possible NULL-ptr deref (vc61):
@/work/benchmarks/SOURCES/wine-0.9.39/tools/sfnt2fnt.c:338
+ ptr os2, returned by function FT_Get_Sfnt_Table dereferenced
without checking - I couldn't find the source of that function.

*** Analyzing /work/benchmarks/BINARY/wine_v0.9.39/tools_winebuild_winebuild.bc
***

Possible NULL-ptr deref (vc5400):
@/work/benchmarks/SOURCES/wine-0.9.39/tools/winebuild/import.c:108
+ seems that table->names can be NULL (or unitialized) at this
line if the test @102 is false, and in the following call chain:
main -> parse_options -> add_extra_ld_symbol -> add_name

Possible NULL-ptr deref (vc5868):
@/work/benchmarks/SOURCES/wine-0.9.39/tools/winebuild/import.c:362
+ seems that ignore_symbols.names can be NULL if test @359 is true.

*** Analyzing /work/benchmarks/BINARY/wine_v0.9.39/tools_winedump_winedump.bc
***

Possible NULL-ptr deref (vc11598):
@/work/benchmarks/SOURCES/wine-0.9.39/tools/winedump/main.c:334
+ malloc-ed ptr buf dereferenced without checking

Possible NULL-ptr deref (vc5636):
@/work/benchmarks/SOURCES/wine-0.9.39/tools/winedump/debug.c:687
+ malloc-ed ptr stabbuff dereferenced without checking

*** Analyzing /work/benchmarks/BINARY/wine_v0.9.39/tools_winegcc_winegcc.bc ***

Possible NULL-ptr deref (vc978):
@/work/benchmarks/SOURCES/wine-0.9.39/tools/winegcc/winegcc.c:467
+ ptr output_file, returned from strdup, dereferenced without checking.
strdup can return NULL.

Possible NULL-ptr deref (vc1013):
@/work/benchmarks/SOURCES/wine-0.9.39/tools/winegcc/utils.c:118
+ starray_dup (utils.c:136) initializes dup with strarray_alloc().
strarray_alloc also sets dup->base to NULL. Then strarray_dup calls
strarray_add (@142), passing it a pointer to dup. If the test in
strarray_add (@113) fails, the function dereferences dup->base directly.
---------------------------------------------------------------

Feedback will be appreciated.

I can't do another round of checking before Aug 1, as I'm travelling.
At that point Calysto will have more knowledge about clib functions,
hopefully resulting in more bugs being reported.

Regards,

--
        Domagoj Babic

http://www.domagoj.info/
        http://www.calysto.org/

[Dan Kegel]

Those seem to all be build-time tools, where problems are not very critical.

On 7/1/07, Domagoj Babic  wrote:
> Hello,
>
> I finally found time to go through Calysto reports for wine. Here are
> the warnings related to applications. For libraries, since Calysto
> doesn't know the calling context, any parameter ptr dereference without
> checking is considered a bug. We can go over those warnings later if
> anyone is really interested.
>
> Here we go:
>
> wine 0.9.39:
>
> Calysto: ??/11
>
> [Please replace ?? with the number of reports that require some
> corrective action, adding assertion is considered a corrective action.
> For every report, leave + if requires corrective action, replace with -
> if it's definitely false positive, and replace with ? if unknown.
> For false positives, please provide explanations. For ?, please ask what's
> unclear. If you discover new bugs that Calysto should have caught,
> please mention that as well.]
>
> *** Analyzing /work/benchmarks/BINARY/wine_v0.9.39/tools_fnt2bdf.bc ***
>
> Possible NULL-ptr deref (vc32):
> @/work/benchmarks/SOURCES/wine-0.9.39/tools/fnt2bdf.c:91
> - main (fnt2bdf.c:548) calls return_data_value (@572), passing it a
>
> pointer to &(pTInfo->type_id), which return_data_value dereferences (@91).
> pTInfo aliases (lpdata+2), and lpdata was initialized with a call to
> get_resource_table (@561), which can set it to NULL (either if malloc
> (@534) returns NULL, or if it is set explicitly to NULL (@540)).
>
> *** Analyzing /work/benchmarks/BINARY/wine_v0.9.39/tools_makedep.bc ***
>
> Possible NULL-ptr deref (vc796):
> @/work/benchmarks/SOURCES/wine-0.9.39/tools/makedep.c:196
> - Ptr p xmalloc-ed (makedep.c:187) dereferenced. Note that although
>
> xmalloc checks pointer (util.c:31), report(R_FATAL, "msg") does not
> exit, but only reports the msg and returns, so p (NULL) will be
> still dereferenced.
>
> *** Analyzing /work/benchmarks/BINARY/wine_v0.9.39/tools_sfnt2fnt.bc ***
>
> Possible NULL-ptr deref (vc47):
> @/work/benchmarks/SOURCES/wine-0.9.39/tools/sfnt2fnt.c:240
> - malloc-ed ptr eblc dereferenced without checking
>
> Possible NULL-ptr deref (vc58):
> @/work/benchmarks/SOURCES/wine-0.9.39/tools/sfnt2fnt.c:321
> - malloc-ed ptr dfCharTable dereferenced without checking
>
> Possible NULL-ptr deref (vc61):
> @/work/benchmarks/SOURCES/wine-0.9.39/tools/sfnt2fnt.c:338
> - ptr os2, returned by function FT_Get_Sfnt_Table dereferenced
>
> without checking - I couldn't find the source of that function.
>
> *** Analyzing /work/benchmarks/BINARY/wine_v0.9.39/tools_winebuild_winebuild.bc
>
> Possible NULL-ptr deref (vc5400):
> @/work/benchmarks/SOURCES/wine-0.9.39/tools/winebuild/import.c:108
> - seems that table->names can be NULL (or unitialized) at this
>
> line if the test @102 is false, and in the following call chain:
> main -> parse_options -> add_extra_ld_symbol -> add_name
>
> Possible NULL-ptr deref (vc5868):
> @/work/benchmarks/SOURCES/wine-0.9.39/tools/winebuild/import.c:362
> - seems that ignore_symbols.names can be NULL if test @359 is true.
>
> *** Analyzing /work/benchmarks/BINARY/wine_v0.9.39/tools_winedump_winedump.bc
>
> Possible NULL-ptr deref (vc11598):
> @/work/benchmarks/SOURCES/wine-0.9.39/tools/winedump/main.c:334
> - malloc-ed ptr buf dereferenced without checking
>
> Possible NULL-ptr deref (vc5636):
> @/work/benchmarks/SOURCES/wine-0.9.39/tools/winedump/debug.c:687
> - malloc-ed ptr stabbuff dereferenced without checking
>
> *** Analyzing /work/benchmarks/BINARY/wine_v0.9.39/tools_winegcc_winegcc.bc ***
>
> Possible NULL-ptr deref (vc978):
> @/work/benchmarks/SOURCES/wine-0.9.39/tools/winegcc/winegcc.c:467
> - ptr output_file, returned from strdup, dereferenced without checking.
>
> strdup can return NULL.
>
> Possible NULL-ptr deref (vc1013):
> @/work/benchmarks/SOURCES/wine-0.9.39/tools/winegcc/utils.c:118
> - starray_dup (utils.c:136) initializes dup with strarray_alloc().
>
> strarray_alloc also sets dup->base to NULL. Then strarray_dup calls
> strarray_add (@142), passing it a pointer to dup. If the test in
> strarray_add (@113) fails, the function dereferences dup->base directly.
>
> Feedback will be appreciated.
>
> I can't do another round of checking before Aug 1, as I'm travelling.
> At that point Calysto will have more knowledge about clib functions,
> hopefully resulting in more bugs being reported.
>
> Regards,
>
> --
>         Domagoj Babic
>     http://www.domagoj.info/
>     http://www.calysto.org/

[Domagoj Babic]

Hi,

On 7/1/07, Dan Kegel  wrote:
> Those seem to all be build-time tools, where problems are not very critical.

So, you are not interested in fixing them?

Which binaries should I focus on?

Regards,

[Dan Kegel]

On 7/1/07, Domagoj Babic  wrote:
> > Those seem to all be build-time tools, where problems are not very critical.
>
> So, you are not interested in fixing them?

Probably not.
> Which binaries should I focus on?

wineserver, i.e. server/*.c, would be the most important
one, I think.  It's a plain old posix application,
more or less.  We also have tons of code in dlls/*/*.c
but that's win32 source code, mostly, so might be
a bit funnier to check.

[Domagoj Babic]

Dan,

My front-end (LLVM) refused to cooperate - I can't compile server and dlls
into the LLVM intermediate form.

I will try again when I update to the new LLVM version in about a month.

Regards,

[Dan Kegel]

OK. Focus just on the server for starters.  See you in a month...

On 7/2/07, Domagoj Babic  wrote:
> Dan,
>
> My front-end (LLVM) refused to cooperate - I can't compile server and dlls
> into the LLVM intermediate form.
>
> I will try again when I update to the new LLVM version in about a month.
>
> Regards,
>
> --
>         Domagoj Babic
>     http://www.domagoj.info/
>     http://www.calysto.org/
>
>
>
> On 7/1/07, Dan Kegel  wrote:
> > On 7/1/07, Domagoj Babic  wrote:
> > > > Those seem to all be build-time tools, where problems are not very critical.
> > >
> > > So, you are not interested in fixing them?
> >
> > Probably not.
> > > Which binaries should I focus on?
> >
> > wineserver, i.e. server/*.c, would be the most important
> > one, I think.  It's a plain old posix application,
> > more or less.  We also have tons of code in dlls/*/*.c
> > but that's win32 source code, mostly, so might be
> > a bit funnier to check.

[Alexandre Julliard]

"Domagoj Babic"  writes:
> Possible NULL-ptr deref (vc796):
> @/work/benchmarks/SOURCES/wine-0.9.39/tools/makedep.c:196
> - Ptr p xmalloc-ed (makedep.c:187) dereferenced. Note that although
>
> xmalloc checks pointer (util.c:31), report(R_FATAL, "msg") does not
> exit, but only reports the msg and returns, so p (NULL) will be
> still dereferenced.

Not a bug, xmalloc does an exit(1).
> *** Analyzing /work/benchmarks/BINARY/wine_v0.9.39/tools_winebuild_winebuild.bc
>
> Possible NULL-ptr deref (vc5400):
> @/work/benchmarks/SOURCES/wine-0.9.39/tools/winebuild/import.c:108
> - seems that table->names can be NULL (or unitialized) at this
>
> line if the test @102 is false, and in the following call chain:
> main -> parse_options -> add_extra_ld_symbol -> add_name

Not a bug, xrealloc never returns NULL.
> Possible NULL-ptr deref (vc5868):
> @/work/benchmarks/SOURCES/wine-0.9.39/tools/winebuild/import.c:362
> - seems that ignore_symbols.names can be NULL if test @359 is true.

Not a bug, names can't be NULL if count isn't 0.
> Possible NULL-ptr deref (vc1013):
> @/work/benchmarks/SOURCES/wine-0.9.39/tools/winegcc/utils.c:118
> - starray_dup (utils.c:136) initializes dup with strarray_alloc().
>
> strarray_alloc also sets dup->base to NULL. Then strarray_dup calls
> strarray_add (@142), passing it a pointer to dup. If the test in
> strarray_add (@113) fails, the function dereferences dup->base directly.

Not a bug, xrealloc never returns NULL.

[Domagoj Babic]

Strange, Calysto should have caught the use of x(malloc|realloc).
Either a bug in Calysto, or I misinterpreted the traces (I did a very
sloppy job analyzing them). Thx for the feedback, I'll try to allocate
more time for the analysis of the next run.

On 7/2/07, Alexandre Julliard  wrote:
> "Domagoj Babic"  writes:
> > Possible NULL-ptr deref (vc796):
> > @/work/benchmarks/SOURCES/wine-0.9.39/tools/makedep.c:196
> > - Ptr p xmalloc-ed (makedep.c:187) dereferenced. Note that although
> >
> > xmalloc checks pointer (util.c:31), report(R_FATAL, "msg") does not
> > exit, but only reports the msg and returns, so p (NULL) will be
> > still dereferenced.
>
> Not a bug, xmalloc does an exit(1).
> > *** Analyzing /work/benchmarks/BINARY/wine_v0.9.39/tools_winebuild_winebuild.bc
> >
> > Possible NULL-ptr deref (vc5400):
> > @/work/benchmarks/SOURCES/wine-0.9.39/tools/winebuild/import.c:108
> > - seems that table->names can be NULL (or unitialized) at this
> >
> > line if the test @102 is false, and in the following call chain:
> > main -> parse_options -> add_extra_ld_symbol -> add_name
>
> Not a bug, xrealloc never returns NULL.
> > Possible NULL-ptr deref (vc5868):
> > @/work/benchmarks/SOURCES/wine-0.9.39/tools/winebuild/import.c:362
> > - seems that ignore_symbols.names can be NULL if test @359 is true.
>
> Not a bug, names can't be NULL if count isn't 0.
> > Possible NULL-ptr deref (vc1013):
> > @/work/benchmarks/SOURCES/wine-0.9.39/tools/winegcc/utils.c:118
> > - starray_dup (utils.c:136) initializes dup with strarray_alloc().
> >
> > strarray_alloc also sets dup->base to NULL. Then strarray_dup calls
> > strarray_add (@142), passing it a pointer to dup. If the test in
> > strarray_add (@113) fails, the function dereferences dup->base directly.
>
> Not a bug, xrealloc never returns NULL.
>
> --
> Alexandre Julliard
> [e-mail]