Closed Bug 174008 Opened 19 years ago Closed 19 years ago

flawfinder warnings in mailnews/movemail [CODE NOT YET USED]

Categories

(MailNews Core :: Movemail, defect)

x86
Windows NT
defect
Not set
normal

Tracking

(Not tracked)

VERIFIED INVALID

People

(Reporter: morse, Assigned: adam)

References

Details

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

flawfinder found 20 warnings in movemail code (3883-3902). 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.

3883) mailnews/movemail/src/movemail.c:104 [4] (format) snprintf: if format 
strings can be influenced by an attacker, they can be exploited. Use a constant 
for the format specification.

3884) mailnews/movemail/src/movemail.c:117 [1] (port) snprintf: On some very old 
systems, snprintf is incorrectly implemented and permits buffer overflows; there 
are also incompatible standard definitions of it. Check it during installation, 
or use something else.

3885) mailnews/movemail/src/movemail.c:148 [4] (buffer) strcpy: does not check 
for buffer overflows. Consider using strncpy or strlcpy.

3886) mailnews/movemail/src/movemail.c:158 [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.

3887) mailnews/movemail/src/movemail.c:160 [4] (format) snprintf: if format 
strings can be influenced by an attacker, they can be exploited. Use a constant 
for the format specification.

3888) mailnews/movemail/src/movemail.c:179 [1] (port) snprintf: On some very old 
systems, snprintf is incorrectly implemented and permits buffer overflows; there 
are also incompatible standard definitions of it. Check it during installation, 
or use something else.

3889) mailnews/movemail/src/movemail.c:181 [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.

3890) mailnews/movemail/src/movemail.c:185 [4] (format) snprintf: if format 
strings can be influenced by an attacker, they can be exploited. Use a constant 
for the format specification.

3891) mailnews/movemail/src/movemail.c:193 [1] (port) snprintf: On some very old 
systems, snprintf is incorrectly implemented and permits buffer overflows; there 
are also incompatible standard definitions of it. Check it during installation, 
or use something else.

3892) mailnews/movemail/src/movemail.c:203 [4] (format) snprintf: if format 
strings can be influenced by an attacker, they can be exploited. Use a constant 
for the format specification.

3893) mailnews/movemail/src/movemail.c:224 [4] (format) snprintf: if format 
strings can be influenced by an attacker, they can be exploited. Use a constant 
for the format specification.

3894) mailnews/movemail/src/movemail.c:264 [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.

3895) mailnews/movemail/src/movemail.c:266 [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.

3896) mailnews/movemail/src/movemail.c:270 [4] (format) snprintf: if format 
strings can be influenced by an attacker, they can be exploited. Use a constant 
for the format specification.

3897) mailnews/movemail/src/movemail.c:279 [4] (format) snprintf: if format 
strings can be influenced by an attacker, they can be exploited. Use a constant 
for the format specification.

3898) mailnews/movemail/src/movemail.c:295 [4] (format) snprintf: if format 
strings can be influenced by an attacker, they can be exploited. Use a constant 
for the format specification.

3899) mailnews/movemail/src/movemail.c:312 [4] (format) snprintf: if format 
strings can be influenced by an attacker, they can be exploited. Use a constant 
for the format specification.

3900) mailnews/movemail/src/movemail.c:327 [4] (format) snprintf: if format 
strings can be influenced by an attacker, they can be exploited. Use a constant 
for the format specification.

3901) mailnews/movemail/src/movemail.c:339 [4] (format) snprintf: if format 
strings can be influenced by an attacker, they can be exploited. Use a constant 
for the format specification.

3902) mailnews/movemail/src/movemail.c:360 [4] (format) snprintf: if format 
strings can be influenced by an attacker, they can be exploited. Use a constant 
for the format specification.
Blocks: 148251
QA Contact: gayatri → stephend
Stuff under mozilla/mailnews/movemail is unused at the moment. It is perhaps
intended as a starting point for new functionality. If anyone intends to base
their work on this existing code would probably be wiser to rewrite it from
scratch anyway...
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → INVALID
Could we cvs remove mailnews/movemail/ dir and files in it? As far as I know,
this code is not built/used, and the movemail support is implemented in:

mailnews/base/src/nsMessengerMigrator.cpp
> Could we cvs remove mailnews/movemail/ dir and files in it? 

just to silence flawfinder?

> As far as I know, this code is not built/used

correct, I believe movemail.c is an external movemail application.

note, mozilla currently only does internal movemail (and that might have
bit-rotted.)

> and the movemail support is implemented in:
> mailnews/base/src/nsMessengerMigrator.cpp

that's just the migration code.  (if you used movemail in 4.x, the code in that
file would set up mozilla for it, too.)

the internal movemail code is here:

/mailnews/local/public/nsIMovemailIncomingServer.idl
/mailnews/local/public/nsIMovemailService.idl
/mailnews/local/src/nsMovemailService.cpp
/mailnews/local/src/nsMovemailIncomingServer.cpp
/mailnews/local/src/nsMovemailIncomingServer.h
/mailnews/local/src/nsMovemailService.h 

if someone was going to add external movemail support to mozilla (people talk
about it, but no one has done it) they could start with movemail.c

if you want to assign this flawfinder bug to me, feel free.

I'll add this info to http://www.mozilla.org/mailnews/movemail/ 
No, it can remain in the tree. I'll leave this bug closed for now, though, since
the code is not built/used. But please mention this in the movemail TODO document.
Summary: flawfinder warnings in mailnews/movemail → flawfinder warnings in mailnews/movemail [CODE NOT YET USED]
[minor point:] I just raised bug#185191 about a broken link to flawfinder at
http://www.mozilla.org/mailnews/movemail/index.html
verified.  http://www.mozilla.org/mailnews/movemail/index.html has been updated
with the relevant info.
Status: RESOLVED → VERIFIED
Product: MailNews → Core
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.