Mailing-List: contact dev-help@httpd.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@httpd.apache.org
Received-SPF: pass (athena.apache.org: domain of boya.sun@case.edu designates
 129.22.105.37 as permitted sender)
From: Boya Sun <boya.sun@case.edu>
To: dev@httpd.apache.org
Cc: ray-yaung.chang@case.edu
Message-ID: <f1ee874c2eb4.2eb4f1ee874c@cwru.edu>
Date: Wed, 14 May 2008 16:40:45 -0400
MIME-Version: 1.0
Content-Language: en
Subject: Re: bugs/inappropriate coding practice discovered by
 interprocedural code analysis for version 2.2.8 of Apache
Priority: normal
In-Reply-To: <482B496A.9060504@force-elite.com>
References: <200805140059347399568@case.edu>
 <200805141131105628758@case.edu> <200805141232263120932@case.edu>
 <200805141359191407554@case.edu> <200805141444195002867@case.edu>
 <482B496A.9060504@force-elite.com>
Content-Type: text/plain; charset=windows-1252
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Dear Paul=2C

Thank you so much for replying to the email=2E It seems that our approach=

is accurate in finding bugs of Category 1=2C but not so accurate for
Category 2 which involves ordering rules=2E Anyway=2C it is a big
encouragement of our work=2C thanks again!

Boya

----- Original Message -----
From=3A Paul Querna =3Cchip=40force-elite=2Ecom=3E
Date=3A Wednesday=2C May 14=2C 2008 4=3A27 pm
Subject=3A Re=3A bugs/inappropriate coding practice discovered by
interprocedural code analysis for version 2=2E2=2E8 of Apache
To=3A dev=40httpd=2Eapache=2Eorg

=3E BOYA SUN wrote=3A
=3E =3E Dear Apache-HTTPD developers=2C
=3E =3E  =

=3E =3E I am a Ph=2ED student in the Software Engineering Research Group =
of =

=3E EECS =

=3E =3E department in Case Western Reserve University=2C under the =

=3E instruction of =

=3E =3E Prof=2E Andy Podgurski=2E In our very recent research=2C we appli=
ed =

=3E =3E inter-procedural code analysis and data mining technique on the =

=3E =3E version 2=2E2=2E8 of Apache project=2C and we have found 6 potent=
ial =

=3E bugs=2C as =

=3E =3E indicated at the end of this email=2E The reason why we did not =

=3E submit =

=3E =3E these bugs to the bug tracking system is that these potential =

=3E bugs have =

=3E =3E not triggered any failure=2C and we cannot be sure whether these =

=3E =3E potential bugs are real bugs or just bad coding practice=2E We ho=
pe =

=3E that =

=3E =3E these potential bugs can help you recognize some real bugs or =

=3E =3E inappropriate coding practice=2E *It would also be greately =

=3E appreciated if =

=3E =3E you can reply to this email after you have gone over the bugs and=
 =

=3E tell =

=3E =3E us whether you have confirmed any of them*=2C since these =

=3E information are =

=3E =3E really valuable for us for testing our current method=2E
=3E =

=3E Compared to previous automated code checking systems=2C the issues =

=3E you =

=3E highlighted have a surprisingly high correctness rate=2E
=3E =

=3E Note that it is also generally best to work against trunk when =

=3E reporting =

=3E bugs=2C if possible=3A
=3E   =3Chttps=3A//svn=2Eapache=2Eorg/repos/asf/httpd/httpd/trunk/=3E
=3E =

=3E =3E *BUG=231*
=3E =3E *Category=3A 1*
=3E =3E *File Name=3A* /httpd-2=2E2=2E8/support/ab=2Ec  =

=3E =3E *Function Name=3A* open=5Fpostfile()
=3E =3E *Buggy Code=3A*
=3E =3E   1901=3A     apr=5Ffile=5Finfo=5Fget(=26finfo=2C APR=5FFINFO=5FN=
ORM=2C postfd)=3B
=3E =3E   1902=3A     postlen =3D (apr=5Fsize=5Ft)finfo=2Esize=3B =

=3E =3E =

=3E =3E *Description=3A* An error might occur if the output of the functi=
on =

=3E =3E apr=5Ffile=5Finfo=5Fget() is not APR=5FSUCCESS=2E The above code =
does not =

=3E check =

=3E =3E the return value of the function=2E
=3E =

=3E =

=3E I agree=2C its a bug=2E
=3E =

=3E Fixed in r656400=3A
=3E https=3A//svn=2Eapache=2Eorg/viewvc=3Fview=3Drev=26revision=3D656400
=3E =

=3E (Rudger=2C you hit commit 30 seconds before me=2C and then I got a =

=3E conflict =

=3E when trying to commit!)
=3E =

=3E =

=3E =3E *BUG=232*
=3E =3E *Category=3A 1 *
=3E =3E *File Name=3A* / httpd-2=2E2=2E8/srclib/apr/file=5Fio/unix/seek=2E=
c
=3E =3E *Function Name=3A* apr=5Ffile=5Fseek()
=3E =3E *Correct Code=3A*
=3E =2E=2E=2E=2E=2E
=3E =3E *Function Name=3A* As follows
=3E =3E *Buggy Code=3A*
=3E =3E 229=3A     apr=5Ffile=5Fflush(=85)=3B // file=3A/server/log=2Ec  =
*Function =

=3E Name=3A* ap=5Freplace=5Fstderr=5Flog()
=3E =3E    424=3A     apr=5Ffile=5Fflush(=85)=3B  // file=3A/server/log=2E=
c  *Function =

=3E Name=3A* ap=5Fopen=5Flogs() =

=3E =3E 683=3A     apr=5Ffile=5Fflush(=85)=3B  // file=3A/server/log=2Ec =
 *Function =

=3E Name=3A* log=5Ferror=5Fcore()
=3E =3E 901=3A     apr=5Ffile=5Fflush(=85)=3B  // file=3A/src/mod=5Fcgi=2E=
c  *Function =

=3E Name=3A* cgi=5Fhandler()
=3E =3E 123=3A     apr=5Ffile=5Fflush(=85)=3B  // file=3A/src/open=2Ec  *=
Function =

=3E Name=3A* apr=5Ffile=5Fclose()
=3E =3E *Description=3A* As shown in the first code fragment =

=3E (apr=5Ffile=5Fseek())=2C the output of apr=5Ffile=5Fflush=5Flocked() =
should be =

=3E checked to determine if it is APR=5FSUCESS=2E By inspecting the sourc=
e =

=3E code of apr=5Ffile=5Fflush()=2C we infer that the output of =

=3E apr=5Ffile=5Fflush() should be checked to determine if it is =

=3E APR=5FSUCCESS also=2E However=2C the output of apr=5Ffile=5Fflush() i=
s not =

=3E checked in the third code fragment=2E  =

=3E =

=3E Yup=2C these are all bugs=2E
=3E =

=3E And no=2C its not okay if flush fails=2C because APR=27s =27flush=27 =
call =

=3E will =

=3E call write() if you are using buffered file IO=2E
=3E =

=3E =3E *BUG=233*
=3E =3E *Category=3A 1*
=3E =3E *File Name=3A* /httpd-2=2E2=2E8/support/htcacheclean=2Ec
=3E =3E *Function Name=3A* process=5Fdir()
=3E =3E *Buggy Code=3A
=3E =3E    534=3A         apr=5Ffile=5Fread=5Ffull(fd=2C =26expires=2C le=
n=2C =26len)=3B
=3E =3E *Description=3A* We found a rule requiring checking if the output=
 =

=3E of =

=3E =3E apr=5Ffile=5Fread=5Ffull() is APR=5FSUCCESS=2E However=2C the abo=
ve code =

=3E ignores =

=3E =3E this check=2E  =

=3E =

=3E =

=3E Bug=2E Fixed in r656401=3A
=3E https=3A//svn=2Eapache=2Eorg/viewvc=3Fview=3Drev=26revision=3D656401
=3E =

=3E =

=3E =3E *BUG=234*
=3E =3E *Category=3A 2*
=3E =3E *File Name=3A* /httpd-2=2E2=2E8/modules/filters/mod=5Finclude=2Ec=
  =

=3E =3E *Function Name=3A* handle=5Finclude
=3E =3E *Buggy Code
=3E =3E   1714=3A         else =7B
=3E =3E   1715=3A             rr =3D ap=5Fsub=5Freq=5Flookup=5Furi(parsed=
=5Fstring=2C r=2C =

=3E f-=3Enext)=3B
=3E =3E   1716=3A         =7D
=3E =3E
=3E =3E *Description=3A* We found a potential rule requiring that the =

=3E =3E ap=5Fdestroy=5Fsub=5Freq() be executed to release the object retu=
rned =

=3E by =

=3E =3E ap=5Fsub=5Freq=5Flookup=5Furi()=2E However=2C ap=5Fdestroy=5Fsub=5F=
req() is not =

=3E called in =

=3E =3E the above code=2E
=3E =

=3E =

=3E Ah=2C your first miss =3A-)
=3E =

=3E This one isn=27t a bug=2E
=3E =

=3E While it is potentially non-optimal=2C due to how Pool cleanups work=2C=
 =

=3E we =

=3E can =27leak=27 the rr variable=2E  When the main request finishes=2C =
the =

=3E sub-pool for RR=2C created from the main request would be recursively=
 =

=3E cleaned up=2E
=3E =

=3E =3E *BUG=235*
=3E =3E *Category=3A 2*
=3E =3E *File Name=3A* /httpd-2=2E2=2E8/modules/http/http=5Frequest=2Ec =

=3E =3E *Function Name=3A* ap=5Finternal=5Ffast=5Fredirect()
=3E =3E *Buggy Code=3A*
=3E =3E    449=3A         ap=5Fremove=5Foutput=5Ffilter(r-=3Eoutput=5Ffil=
ters)=3B
=3E =3E    450=3A         r-=3Eoutput=5Ffilters =3D r-=3Eoutput=5Ffilters=
-=3Enext=3B
=3E =3E    451=3A     =7D
=3E =3E *Description=3A* We found a potential rule requiring that =

=3E =3E ap=5Fpass=5Fbrigade() be called after the execution of =

=3E =3E ap=5Fremove=5Foutput=5Ffilter()=2E However=2C the above code does=
 not =

=3E follow this =

=3E =3E potential rule=2E
=3E =

=3E Nope=2C ap=5Fpass=5Fbrigade is commonly used in that way=2C for examp=
le =

=3E when a =

=3E filter is done=2C it removes itself=2C and then passes the content do=
wn =

=3E the =

=3E chain=2C but they are not dependent on each other=2E
=3E =

=3E =3E *Category=3A 1*
=3E =3E *File Name=3A* /httpd-2=2E2=2E8/srclib/srclib/apr-
=3E util/xml/expat/lib/xmlparse=2Ec  =

=3E =3E *Function Name=3A* doProlog()
=3E =3E *Buggy Code=3A*
=3E =3E 2670=3A         ENTITY *entity =3D (ENTITY *)lookup(=26dtd=2Epara=
mEntities=2C
=3E =3E =

=3E =3E   2671=3A                                                        =
   =

=3E                               externalSubsetName=2C
=3E =3E   2672=3A                                                        =
   =

=3E                               0)=3B
=3E =3E   2673=3A           if =

=3E (!externalEntityRefHandler(externalEntityRefHandlerArg=2C=3E   2674=3A=
    =

=3E                                                                    =

=3E                   0=2C
=3E =3E   2675=3A                                                        =
   =

=3E                                entity-=3Ebase=2C
=3E =3E   2676=3A                                                        =
   =

=3E                                entity-=3EsystemId=2C
=3E =3E   2677=3A                                                        =
   =

=3E                                entity-=3EpublicId))
=3E =3E *Description=3A* The function lookup() might return NULL=2E In th=
e =

=3E above =

=3E =3E code=2C the fields of the variable /entity/=2C returned by look()=
=2C =

=3E are =

=3E =3E accessed without checking if it is NULL=2E    =

=3E =

=3E =

=3E This one looks like a bug=2E=2E but in expat=2E=2E=2E I guess we shou=
ld look =

=3E at =

=3E reporting it upstream=2E
=3E =

=3E -Paul
=3E