472032 - [win64] sizeof(long) != sizeof(void*) assertion in nsScriptSecurityManager.cpp

Status: RESOLVED FIXED

(Core :: Security: CAPS, defect)

Description

[:Mook]

See URL.  This assertion gets triggered because sizeof(long) = 4, sizeof(void*) = 8.

See bug 119646 comment 9 to comment 20.  It's guarding union SecurityLevel at http://mxr.mozilla.org/mozilla-central/source/caps/include/nsScriptSecurityManager.h?rev=9f497b1505d2&mark=149#144

This triggers on startup of any win64 build.  I have no idea how the other win64 people manage to not see it...

###!!! ASSERTION: long and void* have different lengths on this platform. This may cause a security failure.: 'sizeof(long) == sizeof(void*)', file .../mozilla/src/caps/src/nsScriptSecurityManager.cpp, line 3220

Comment 1

[Makoto Kato [:m_kato]]

According to CVS log (http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&root=/cvsroot&subdir=mozilla/caps/src&command=DIFF_FRAMESET&file=nsScriptSecurityManager.cpp&rev2=1.162&rev1=1.161), this was checked in bug 119646.

I believe that this assertion code is not necessary.

Comment 2

[:Mook]

Attachment: use PRWord, not long, and drop assertion

insert of asserting, just use the type that's guaranteed to be sizeof(void*) instead.

Thanks to timeless for helping tracking down the initial reason.

Comment 3

[Daniel Veditz [:dveditz]]

Comment on attachment 363527 [details] [diff] [review]
use PRWord, not long, and drop assertion

r/sr=dveditz

Would it be better to leave the assertion and change it to sizeof(PRWord) instead? It seems to be the right thing to use here (jst and sicking agree) but I'm slightly concerned by the comment in prtypes.h saying it's deprecated. Updating the assertion might help if they stop maintaining it for future platforms. If you do leave the assertion change the text to end "This may cause a security failure with the SecurityLevel union."

Comment 4

[:Mook]

Attachment: address comment 3 [Checkin: Comment 6]

carrying forward r/sr

(In reply to comment #3)
> (From update of attachment 363527 [details] [diff] [review])

> Would it be better to leave the assertion and change it to sizeof(PRWord)
> instead? 
Done.

> I'm slightly concerned by the comment in prtypes.h saying it's deprecated.
Unfortunately, unless NSPR decides to provide a type that actually is guaranteed to be sizeof(void*), we're stuck with it.  C99 got intptr_t, so having such a type must make sense to somebody :)

Comment 5

[Peter Van der Beken [:peterv]]

(In reply to comment #4)
> Unfortunately, unless NSPR decides to provide a type that actually is
> guaranteed to be sizeof(void*)

See PRUptrdiff.

Comment 6

[Serge Gautherie (:sgautherie)]

Comment on attachment 364282 [details] [diff] [review]
address comment 3
[Checkin: Comment 6]


http://hg.mozilla.org/mozilla-central/rev/04579855a2b7

Comment 7

[Serge Gautherie (:sgautherie)]

(In reply to comment #5)
> See PRUptrdiff.

Ah well: reopen as needed.

Comment 8

[:Mook]

Attachment: PRUptrdiff

Bah, sorry, I thought I had already attached this, then my internet access went down :(

Here's a trivial version to use PRUptrdiff, if you think it's necessary?  (Feel free to clear the requests out if not.)

For what it's worth, I used PRWord because it's not a pointer difference (as in, intptr_t vs ptrdiff_t)

Comment 9

[Peter Van der Beken [:peterv]]

Comment on attachment 366299 [details] [diff] [review]
PRUptrdiff

>diff --git a/caps/include/nsScriptSecurityManager.h b/caps/include/nsScriptSecurityManager.h

>+    PRUptrdiff level;
>+    char*     capability;

Nit: alignment is not right.

>+    NS_ASSERTION(sizeof(PRUptrdiff) == sizeof(void*),

Make this a PR_STATIC_ASSERT.