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

Status: RESOLVED FIXED

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

Description

[timeless]

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

Comment 1

[Johnny Stenback (:jst)]

Attachment: Fix.

Comment 2

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

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.

Comment 3

[Johnny Stenback (:jst)]

Attachment: Better fix (triggered by Jonas' question)

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.

Comment 4

[Johnny Stenback (:jst)]

Fixed.

http://hg.mozilla.org/mozilla-central/rev/aad0d687a764