Closed Bug 1530240 Opened 10 months ago Closed 9 months ago

Fix issues found by coverity in TestAUSHelper.cpp

Categories

(Toolkit :: Application Update, enhancement)

59 Branch
enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla67
Tracking Status
firefox67 --- fixed

People

(Reporter: robert.strong.bugs, Assigned: robert.strong.bugs)

Details

Attachments

(1 file, 1 obsolete file)

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);

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

Attachment #9046268 - Attachment description: Bug 1530240 - TestAUSHelper.cpp truncation checks and coverity fixes. r?mhowell → Bug 1530240 - TestAUSHelper.cpp coverity fixes. r?mhowell

I'm limiting this bug to just the issues found by coverity.

Summary: Fix truncation issues and issues found by coverity in TestAUSHelper.cpp → Fix issues found by coverity in TestAUSHelper.cpp
Attachment #9046268 - Attachment description: Bug 1530240 - TestAUSHelper.cpp coverity fixes. r?mhowell → Bug 1530240 - (test code only) - TestAUSHelper.cpp coverity fixes. r?mhowell

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

Attachment #9046557 - Attachment is obsolete: true
Pushed by rstrong@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e5e2ac9bb33b
(test code only) - TestAUSHelper.cpp coverity fixes. r=mhowell
Pushed by rstrong@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c04aa3d7aa8e
(test code only) - TestAUSHelper.cpp coverity fixes. r=mhowell
Status: ASSIGNED → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67
Flags: needinfo?(robert.strong.bugs)
You need to log in before you can comment on or make changes to this bug.