Closed Bug 173733 Opened 22 years ago Closed 18 years ago

flawfinder warnings in netwerk

Categories

(Core :: Networking, defect)

x86
Windows NT
defect
Not set
normal

Tracking

()

RESOLVED WORKSFORME
Future

People

(Reporter: morse, Assigned: dougt)

References

Details

(Keywords: helpwanted)

Heikki ran flawfinder (http://www.dwheeler.com/flawfinder) on Mozilla 1.0.1 
branch.

flawfinder found 18 warnings in netwerk code (2121-2138). 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.

2121) netwerk/base/src/nsAsyncStreamListener.cpp:243 [2] (buffer) sprintf: does 
not check for buffer overflows. Use snprintf or vsnprintf. Risk is low because 
the source has a constant maximum length.

2122) netwerk/cache/src/nsDiskCacheMap.cpp:801 [2] (buffer) sprintf: does not 
check for buffer overflows. Use snprintf or vsnprintf. Risk is low because the 
source has a constant maximum length.

2123) netwerk/cache/src/nsDiskCacheMap.cpp:836 [2] (buffer) sprintf: does not 
check for buffer overflows. Use snprintf or vsnprintf. Risk is low because the 
source has a constant maximum length.

2124) netwerk/dns/daemon/nsDnsAsyncLookup.cpp:118 [3] (buffer) getenv: 
Environment variables are untrustable input if they can be set by an attacker. 
They can have any content and length, and the same variable can be set more than 
once.. Check environment variables carefully before using them.

2125) netwerk/dns/daemon/nsDnsAsyncLookup.cpp:314 [4] (buffer) strcpy: does not 
check for buffer overflows. Consider using strncpy or strlcpy.

2126) netwerk/dns/daemon/nsDnsAsyncLookup.cpp:526 [4] (buffer) strcpy: does not 
check for buffer overflows. Consider using strncpy or strlcpy.

2127) netwerk/dns/src/nsDnsService.cpp:947 [3] (buffer) getenv: Environment 
variables are untrustable input if they can be set by an attacker. They can have 
any content and length, and the same variable can be set more than once.. Check 
environment variables carefully before using them.

2128) netwerk/mime/src/nsXMLMIMEDataSource.cpp:393 [2] (buffer) sprintf: does 
not check for buffer overflows. Use snprintf or vsnprintf. Risk is low because 
the source has a constant maximum length.

2129) netwerk/mime/src/nsXMLMIMEDataSource.cpp:402 [2] (buffer) sprintf: does 
not check for buffer overflows. Use snprintf or vsnprintf. Risk is low because 
the source has a constant maximum length.

2130) netwerk/protocol/ftp/src/nsFTPChannel.cpp:697 [4] (race) access: this 
usually indicates a security flaw. If an attacker can change anything along the 
path between the call to access() and the file's actual use (e.g., by moving 
files), the attacker can exploit the race condition. Set up the correct 
permissions (e.g., using setuid()) and try to open the file directly.

2131) netwerk/protocol/ftp/src/nsFtpConnectionThread.cpp:2057 [4] (race) access: 
this usually indicates a security flaw. If an attacker can change anything along 
the path between the call to access() and the file's actual use (e.g., by moving 
files), the attacker can exploit the race condition. Set up the correct 
permissions (e.g., using setuid()) and try to open the file directly.

2132) netwerk/protocol/ftp/src/nsFtpConnectionThread.cpp:2058 [4] (race) access: 
this usually indicates a security flaw. If an attacker can change anything along 
the path between the call to access() and the file's actual use (e.g., by moving 
files), the attacker can exploit the race condition. Set up the correct 
permissions (e.g., using setuid()) and try to open the file directly.

2133) netwerk/protocol/ftp/src/nsFtpConnectionThread.cpp:2062 [4] (race) access: 
this usually indicates a security flaw. If an attacker can change anything along 
the path between the call to access() and the file's actual use (e.g., by moving 
files), the attacker can exploit the race condition. Set up the correct 
permissions (e.g., using setuid()) and try to open the file directly.

2134) netwerk/protocol/http/src/nsHttpChannel.cpp:3135 [4] (race) access: this 
usually indicates a security flaw. If an attacker can change anything along the 
path between the call to access() and the file's actual use (e.g., by moving 
files), the attacker can exploit the race condition. Set up the correct 
permissions (e.g., using setuid()) and try to open the file directly.

2135) netwerk/protocol/http/src/nsHttpChannel.cpp:3139 [4] (race) access: this 
usually indicates a security flaw. If an attacker can change anything along the 
path between the call to access() and the file's actual use (e.g., by moving 
files), the attacker can exploit the race condition. Set up the correct 
permissions (e.g., using setuid()) and try to open the file directly.

2136) netwerk/protocol/http/src/nsHttpChannel.cpp:3149 [4] (race) access: this 
usually indicates a security flaw. If an attacker can change anything along the 
path between the call to access() and the file's actual use (e.g., by moving 
files), the attacker can exploit the race condition. Set up the correct 
permissions (e.g., using setuid()) and try to open the file directly.

2137) netwerk/streamconv/converters/nsHTTPChunkConv.cpp:150 [2] (buffer) 
sprintf: does not check for buffer overflows. Use snprintf or vsnprintf. Risk is 
low because the source has a constant maximum length.

2138) netwerk/streamconv/src/nsAppleFileDecoder.h:89 [4] (race) access: this 
usually indicates a security flaw. If an attacker can change anything along the 
path between the call to access() and the file's actual use (e.g., by moving 
files), the attacker can exploit the race condition. Set up the correct 
permissions (e.g., using setuid()) and try to open the file directly.
Blocks: 148251
morse: is this part of a security audit, or just some dogfood data you found
while working.

If you know who should get this, go ahead and assign or cc appropriately.
Keywords: helpwanted
It's part of the security audit.  With 7,000 + flawfinder warnings, I can't go
tracking down the correct owner of every one of them.  If you know who these
should go to and can reassign, that would be appreciated.
> 2124) netwerk/dns/daemon/nsDnsAsyncLookup.cpp:118 [3] (buffer) getenv: 

Our use of it is fairly safe; we just pass it along to inet_addr()

> 2130) netwerk/protocol/ftp/src/nsFTPChannel.cpp:697 [4] (race) access: this 

This is not a call to access(); this is a variable named "access".
Same applies to every other warning about "access" in that list.

The sprintf()/strcpy() warnings look like false alarms for the most part, but
using snprintf/strncpy can't hurt.
over to me.
Assignee: new-network-bugs → dougt
Target Milestone: --- → Future
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).
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.