Fix issues found by coverity in TestAUSHelper.cpp
Categories
(Toolkit :: Application Update, enhancement)
Tracking
()
Tracking | Status | |
---|---|---|
firefox67 | --- | fixed |
People
(Reporter: robert.strong.bugs, Assigned: robert.strong.bugs)
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
text/x-phabricator-request
|
Details |
The approach for dealing with truncation in TestAUSHelper.cpp will be used to decide on the best approach for dealing with truncation in the client code as well.
CID 16675 (#2 of 2): Unchecked return value from library (CHECKED_RETURN)5. check_return: Calling mkdir(path, 493U) without checking return value. This library function may fail and return an error code.
mkdir(path, 0755);
CID 16969 (#2 of 2): Unchecked return value from library (CHECKED_RETURN)6. check_return: Calling chmod(path, 493U) without checking return value. This library function may fail and return an error code.
chmod(path, 0755);
static bool CheckMsg(const NS_tchar *path, const char *expected) {
CID 17966 (#1 of 1): Time of check time of use (TOCTOU)1. fs_check_call: Calling function access to perform check on path.
2. Condition access(path, 0), taking false branch.
if (NS_taccess(path, F_OK)) {
return false;
}
3. toctou: Calling function fopen that uses path after a check function. This can cause a time-of-check, time-of-use race condition.
FILE *inFP = NS_tfopen(path, NS_T("rb"));
CID 18445 (#1 of 1): Copy into fixed size buffer (STRING_OVERFLOW)3. fixed_size_dest: You might overrun the 1024-character fixed-size string exePath by copying argv[0] without checking the length.
strcpy(exePath, argv[0]);
CID 18565 (#1 of 1): Unbounded source buffer (STRING_SIZE)4. string_size: Passing string argv[0] of unknown size to strcpy, which expects a string of a particular size.
strcpy(exePath, argv[0]);
struct stat ss;
CID 18959 (#1 of 1): Unchecked return value from library (CHECKED_RETURN)7. check_return: Calling lstat(argv[2], &ss) without checking return value. This library function may fail and return an error code.
lstat(argv[2], &ss);
![]() |
Assignee | |
Comment 1•6 years ago
|
||
Change NS_tsnprintf into a function that returns success and failure and change consumers to check success and failure
Remove unnecessary access check in CheckMsg
Change call to strcpy to NS_tsnprintf so truncated values aren't used
Check return value of mkdir, rmdir, chmod, lstat, and unlink when the program should fail and cast to void when it the program should continue
Remove CXXFLAGS += ['-Wno-format-truncation'] from the moz.build for TestAUSHelper.cpp
Updated•6 years ago
|
![]() |
Assignee | |
Comment 2•6 years ago
|
||
I'm limiting this bug to just the issues found by coverity.
Updated•6 years ago
|
![]() |
Assignee | |
Comment 3•6 years ago
|
||
Remove unnecessary access check in CheckMsg
Change call to strcpy to NS_tsnprintf
Add checks for the return value of mkdir, rmdir, chmod, lstat, and unlink
Updated•6 years ago
|
Pushed by rstrong@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/e5e2ac9bb33b (test code only) - TestAUSHelper.cpp coverity fixes. r=mhowell
Comment 5•6 years ago
|
||
Backed out changeset e5e2ac9bb33b (Bug 1530240) for causing build bustages in updatedefines.h CLOSED TREE
Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=230448832&repo=mozilla-inbound&lineNumber=27239
Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/3cf44f4ad5ce21fecf0374bb99cc1c21787cd2cc
Pushed by rstrong@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/c04aa3d7aa8e (test code only) - TestAUSHelper.cpp coverity fixes. r=mhowell
Comment 7•6 years ago
|
||
bugherder |
![]() |
Assignee | |
Updated•5 years ago
|
Description
•