Closed
Bug 1428568
Opened 6 years ago
Closed 6 years ago
Stop using GetNativePath in libjar
Categories
(Core :: Networking: JAR, defect, P2)
Core
Networking: JAR
Tracking
()
RESOLVED
FIXED
mozilla60
Tracking | Status | |
---|---|---|
firefox60 | --- | fixed |
People
(Reporter: emk, Assigned: emk)
References
Details
(Whiteboard: [necko-triaged])
Attachments
(1 file)
(In reply to Honza Bambas (:mayhemer) from bug 685236 comment #65) > > if (rv == NS_OK || !outFile) > > return rv; > > if (...) { ... } please, when you are here Fixed locally. > > #ifdef XP_WIN > > DeleteFileW(outFile->NativePath().get()); > > #else > > PR_Delete(outFile->NativePath().get()); > > #endif > > > > I think deleting |outname| always was a huge and probably even potentially > security sensitive bug! > > I don't understand why it's even passed in here, since it always has been a > const c-string held by nsAutoCString. That must never be deleted directly! > > This weird code has been introduced in bug 214672 [1][2], probably slipped > among those twenty three versions of that huge patch... > > hence, please remove passing the path down as an arg to ExtractFile > altogether. PR_Delete does not deleting the passed pointer. It is a portable version of unlink. Maybe the function name should have been PR_DeleteFile. I will add a generic wrapper instead of adding #ifdefs everywhere as some reviewers requested in bug 685236.
Updated•6 years ago
|
Priority: -- → P2
Whiteboard: [necko-triaged]
Comment 1•6 years ago
|
||
(In reply to Masatoshi Kimura [:emk] from comment #0) > PR_Delete does not deleting the passed pointer. It is a portable version of > unlink. Maybe the function name should have been PR_DeleteFile. > I will add a generic wrapper instead of adding #ifdefs everywhere as some > reviewers requested in bug 685236. Ah! I misread that as some other form of "delete-ptr" function. Right. Thanks.
Comment hidden (mozreview-request) |
Comment 3•6 years ago
|
||
mozreview-review |
Comment on attachment 8949023 [details] Bug 1428568 - Stop using GetNativePath in libjar. https://reviewboard.mozilla.org/r/218418/#review224888 check all places are updated ::: modules/libjar/nsZipArchive.cpp:518 (Diff revision 1) > if (aFd) { > PR_Close(aFd); > - if (rv != NS_OK) > - PR_Delete(outname); > + if (rv == NS_OK || !outFile) { > + return rv; > + } > + outFile->Remove(false); rather just: if (NS_FAILED(rv) && outFile) { outFile->Remove(false); }
Attachment #8949023 -
Flags: review?(honzab.moz) → review+
Comment hidden (mozreview-request) |
Assignee | ||
Updated•6 years ago
|
Comment hidden (mozreview-request) |
Assignee | ||
Comment 6•6 years ago
|
||
mozreview-review-reply |
Comment on attachment 8949023 [details] Bug 1428568 - Stop using GetNativePath in libjar. https://reviewboard.mozilla.org/r/218418/#review224888 > rather just: > > if (NS_FAILED(rv) && outFile) { > outFile->Remove(false); > } Fixed.
Pushed by VYV03354@nifty.ne.jp: https://hg.mozilla.org/integration/autoland/rev/0bb9c745a9df Stop using GetNativePath in libjar. r=mayhemer
Comment 8•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/0bb9c745a9df
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox60:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
You need to log in
before you can comment on or make changes to this bug.
Description
•