173428 - flawfinder warnings in libjar

Status: RESOLVED WORKSFORME

(Core :: Networking: JAR, defect)

Description

[Stephen P. Morse]

I run flawfinder (http://www.dwheeler.com/flawfinder) on Mozilla 1.0.1 branch.

flawfinder found 6 warnings in libjar code (1105-1110). Go through
that list and for each warning:

* If it is false positive, comment here why it is not an issue
* If it is a real issue, make patch for it here and let's get them checked in

In addition to checking the branch, also check the trunk.

1105) modules/libjar/nsJAR.cpp:287 [5] (race) chmod: this accepts filename 
arguments; if an attacker can move those files, a race condition results. . Use 
fchmod( ) instead.

1106) modules/libjar/nsZipArchive.cpp:676 [5] (race) chmod: this accepts 
filename arguments; if an attacker can move those files, a race condition 
results. . Use fchmod( ) instead.

1107) modules/libjar/nsZipArchive.cpp:1779 [2] (buffer) sprintf: does not check 
for buffer overflows. Use snprintf or vsnprintf. Risk is low because the source 
has a constant maximum length.

1108) modules/libjar/nsZipArchive.cpp:1792 [2] (buffer) sprintf: does not check 
for buffer overflows. Use snprintf or vsnprintf. Risk is low because the source 
has a constant maximum length.

1109) modules/libjar/nsZipArchive.cpp:1825 [4] (buffer) sprintf: does not check 
for buffer overflows. Use snprintf or vsnprintf.

1110) modules/libjar/zipstub.h:41 [4] (buffer) strcpy: does not check for buffer 
overflows. Consider using strncpy or strlcpy.

Comment 1

[Stephen P. Morse]

One more flawfinder warning -- 3921

3921) modules/libjar/nsZipArchive.cpp:676 [5] (race) chmod: this accepts
filename arguments; if an attacker can move those files, a race condition
results. . Use fchmod( ) instead.

Comment 2

[Heikki Toivonen (remove -bugzilla when emailing directly)]

1107-1109 are safe because the buffer is big enough.

1110 is safe, define strcpy to PL_strcpy and all the usage of that is safe in libjar

I have NOT checked 1105-1106, 3921 is a duplicate of 1106.

Comment 3

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Closing all open flawfinder bugs as WORKSFORME because we now have much better tools that do the same (well, better) kind of analysis (Coverity, Klocwork).