Bugs in Wine research

[tomecek at cs.aau.dk]

Hi,
we are students from Department of computer science at Aalborg  
university. During this semester we were working on project on static  
analysis using the Coccinelle tool  
(http://www.emn.fr/x-info/coccinelle/) and Flawfinder  
(http://en.wikipedia.org/wiki/Flawfinder). We decided to search for  
bugs in Wine source code.

Our aim was to find as many bugs in Wine source code as possible but  
on the other hand to find real and dangerous ones. We solved some of  
the Janitorial projects and beside that we searched for common bugs.

We decided to deal with 2 of Janitorial Projects: Ignored return  
values and Memory leaks. Beside that searched for memory allocation  
without NULL checking. Then we ran some more scripts searching for not  
unlocking memory objects after locking them and searching for untested  
file descriptors. We also ran two tests looking for not dangerous  
problems: searching for unused variables and searching for pointers  
comparison to 0 instead of NULL.

All the test were run on Wine 0.9.57 version.

In http://www.cs.aau.dk/~tomecek/scripts.tar.gz we present the list of  
the most obvious bugs in 'diff' format and Coccinelle 'cocci' scripts  
in Semantic Patch Language (SmPL) used for bug searching.

* Ignored return variables
We found 115 function callings without checking the returned value  
which we considered to be possibly dangerous. In many cases we think  
that the behaviour could be dangerous but we need some information  
about the code.

In 'ignored_return_values.diff' we present some examples of code where  
the returned value should not be ignored but it is so.

* Unused variables
This was one of the Janitorial Projects. This issue is not dangerous  
but it helps to clean the code and make it more readable. All of the  
variables could be removed using \textit{diff} file. We found 18  
matches and all of them were true positives.

Beside that we find some possible security bug with 'Flawfinder'. Here  
is the output:

wine-0.9.57/programs/taskmgr/perfdata.c:292:  [4] (access)  
ImpersonateLoggedOnUser:
   If this call fails, the program could fail to drop heightened
   privileges. Make sure the return value is checked, and do not continue if a
   failure is reported.

* NULL comparison
Sometimes developers use comparison of pointers to zero instead of  
NULL. This is not critical bug but the \textit{gcc} compiler warns  
against such using.

In this case we were able to change all the zero comparison to NULL  
variant using Coccinelle. We found 176 matches and all of them were  
corrected automatically.

* Memory allocation
Using both functions for memory allocation - 'malloc' and 'HeapAlloc'  
- requires NULL checking of the returned value to be sure that the  
allocation was done correctly. We matched 598 possible bugs.

In 'memAlloc.diff' we send to you 10 examples of bugs we have found.

* Memory leaks
This searching was part of one of the Janitorial Projects. We were  
looking for memory allocations where the pointer to the allocated  
memory is lost or forgotten without freeing the memory.

We matched 13 of them as true positives.

In 'memLeaks.diff' we send to you 5 examples of most obvious problems.

* Object locking
We were searching for locking object with 'GlobalLock()' without  
'GlobalUnLock()' for unlocking. If the object was locked then it has  
to be unlocked to prevent deadlocks when some process or thread is  
waiting for unlocking of this object. We have 2 true positives.

* Descriptors
In this test we were looking for unchecked descriptors returned by  
fopen(), fdopen(), freopen(), open(), creat() and pipe() or for wrong  
check of this descriptors. fopen(), fdopen() and freopen() functions  
return in the case of failure 0 but the rest of these functions return  
-1. Sometimes programmers use it in wrong way and sometimes they do  
not check it anyway.

We matched 3 wrong usings.

If you are interested in our work or if you want the full list of bugs  
we found let us know.

Regards,
Bertrand Dechoux,
Youenn Corre,
Jaroslav Tomecek

----------------------------------------------------------------
This message was sent using IMP, the Internet Messaging Program.