173790 - flawfinder warnings in uriloader

Status: VERIFIED INVALID

Description

[Stephen P. Morse]

flawfinder found 2 warnings in uriloader code (2209-2210). 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.

2209) uriloader/exthandler/nsExternalHelperAppService.cpp:1053 [3] (random) 
srand: This function is not sufficiently random for security-related functions 
such as key and nonce creation. use a more secure technique for acquiring random 
values.

2210) uriloader/exthandler/mac/nsDecodeAppleFile.h:92 [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.

Comment 1

[Boris Zbarsky [:bzbarsky, bz on IRC]]

2209) -- this use of srand() is not to generate a key or nonce -- it's just to
generate a filename different from the "default" one using a non-deterministic
algorithm.  The usual srand() issue -- strong correlation between succeeding
values -- is not a problem here since the attacker has no access to _any_ of the
values.

2210) -- this is not a call to access(), this is a variable named access.

What's the correct resolution here?  worksforme?  invalid?

Comment 2

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

If all of the issues were false positives, we have been using invalid. Marking
as such. Thanks, Boris.

Comment 3

[Jimmy Lee]

Catching up.  Reopen if you disagree.  Marking Verified!