662126 - Audit ConfirmEx callers to ensure that aCheckState is not an uninitialized PRBool

Status: RESOLVED FIXED

(Core :: General, defect)

Description

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

After hitting Bug 662125 in a debug build and finding the cause I found, without looking particularly hard,

http://hg.mozilla.org/mozilla-central/annotate/57bedceef898/netwerk/protocol/http/nsHttpChannelAuthProvider.cpp#l1185
http://hg.mozilla.org/mozilla-central/annotate/57bedceef898/security/manager/ssl/src/nsCrypto.cpp#l2940

There may be others as well.

Comment 1

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Looking at the patch for Bug 495618 would probably be a decent place to start.

Comment 2

[Jonathan Protzenko [:protz]]

Here's at least four call sites that should be fixed:
- http://mxr.mozilla.org/comm-central/source/mozilla/caps/src/nsScriptSecurityManager.cpp#2803 (the last parameter of CheckConfirmDialog is passed to ConfirmEx)
- http://mxr.mozilla.org/comm-central/source/mozilla/security/manager/ssl/src/nsCrypto.cpp#2941
- http://mxr.mozilla.org/comm-central/source/mozilla/toolkit/xre/nsAppRunner.cpp#1813
- http://mxr.mozilla.org/comm-central/source/mozilla/netwerk/protocol/http/nsHttpChannelAuthProvider.cpp#1186

(Looking for more...)

Comment 3

[Jonathan Protzenko [:protz]]

Attachment: Fix all four call sites that I've found

This is all I could come up with. I've checked every instance of ConfirmEx in C++ code, and for each function that ends up calling ConfirmEx with the penultimate parameter being one of its own parameters, I've checked that function's own call sites.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 537608 [details] [diff] [review]
Fix all four call sites that I've found

Looks good, thanks!

Comment 5

[Jonathan Protzenko [:protz]]

http://hg.mozilla.org/mozilla-central/rev/8b3b1f9c132e