Closed Bug 550905 Opened 10 years ago Closed 10 years ago

nsGlobalWindow::CheckSecurityLeftAndTop uses winLeft, winTop, winWidth, winHeight unitialized if !treeOwner

Categories

(Core :: DOM: Core & HTML, defect)

x86
Linux
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: timeless, Assigned: jst)

References

(Blocks 1 open bug, )

Details

(Keywords: coverity)

Attachments

(1 file, 1 obsolete file)

3621 nsGlobalWindow::CheckSecurityLeftAndTop(PRInt32* aLeft, PRInt32* aTop)
3635     PRInt32 winLeft, winTop, winWidth, winHeight;

3646     if (treeOwner)
3647       treeOwner->GetPositionAndSize(&winLeft, &winTop, &winWidth, &winHeight);

3651     winLeft   = DevToCSSIntPixels(winLeft);
3652     winTop    = DevToCSSIntPixels(winTop);
3653     winWidth  = DevToCSSIntPixels(winWidth);
3654     winHeight = DevToCSSIntPixels(winHeight);
Attached patch Fix. (obsolete) — Splinter Review
Assignee: nobody → jst
Attachment #431214 - Flags: superreview?(jonas)
Attachment #431214 - Flags: review?(jonas)
Attachment #431214 - Flags: superreview?(jonas)
Attachment #431214 - Flags: superreview+
Attachment #431214 - Flags: review?(jonas)
Attachment #431214 - Flags: review+
Comment on attachment 431214 [details] [diff] [review]
Fix.

>diff --git a/dom/base/nsGlobalWindow.cpp b/dom/base/nsGlobalWindow.cpp
>--- a/dom/base/nsGlobalWindow.cpp
>+++ b/dom/base/nsGlobalWindow.cpp
>@@ -3624,7 +3624,10 @@ nsGlobalWindow::CheckSecurityLeftAndTop(
> 
>   // Check security state for use in determing window dimensions
> 
>-  if (!nsContentUtils::IsCallerTrustedForWrite()) {
>+  nsCOMPtr<nsIBaseWindow> treeOwner;
>+  GetTreeOwner(getter_AddRefs(treeOwner));
>+
>+  if (!nsContentUtils::IsCallerTrustedForWrite() && treeOwner) {

Should you set the values to 0 if treeOwner is null, just in case?

r/sr=me with that fixed or explained.
Went to 0 out the out params in the case where treeOwner is null, and realized that this should be changed around a bit to do less if checks and be a bit cleaner, so I did that as well. This will do nothing if the caller is trusted enough, if it's not, and we have a treeOwner and screen, do the checks, and if not, return 0 for *aLeft and *aTop.
Attachment #431214 - Attachment is obsolete: true
Attachment #431507 - Flags: review?(jonas)
Fixed.

http://hg.mozilla.org/mozilla-central/rev/aad0d687a764
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.