Closed Bug 282784 Opened 20 years ago Closed 19 years ago

[FIXr]Warning appears twice when submitting unencrypted form data


(Core Graveyard :: Security: UI, defect, P1)



(Not tracked)



(Reporter: djcater+bugzilla, Assigned: bzbarsky)




(1 file, 3 obsolete files)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8b) Gecko/20050218 Firefox/1.0+
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8b) Gecko/20050218 Firefox/1.0+

On certain pages with forms, when submitting them, the warning about submitting
unencrypted form data is shown. When 'OK' is pressed, the dialogue disappears,
but then another exact copy reappears. After pressing 'OK' a second time, the
form is submitted. For this to occur, security.warn_submit_insecure must be set
to true, and security.warn_submit_insecure.show_once must be set to false.

Reproducible: Always

Steps to Reproduce:
1. Check your settings in about:config as specified above.
2. Load up the testcase.
3. Submit the form.
4. Select 'OK' in the subsequent warning.
Actual Results:  
The warning appears again. Selecting OK a second time submits the form and no
further warnings are displayed.

Expected Results:  
Only shown the warning once, then submitted the form after pressing 'OK' once.

This occurs with a default profile except for the 2 settings specified above.

Observed with latest trunk in Gentoo Linux and Windows XP.

Attaching testcase...
Attached file Testcase (obsolete) —
Testcase. Follow the instructions in the first post to observe the bug.
Attached file Testcase (obsolete) —
Testcase. Follow instructions in first post to observe bug.
Attachment #174746 - Attachment is obsolete: true
OK, the testcases won't demonstrate the bug because the attachments are on this
secure server. Download the first attachment, and run it locally.
Worksforme with a Seamonkey build from the exact same day.  Just tested with a
Firefox and SeaMonkey built from the same tree, and this is Firefox-only.

Leaving in here for now in case this is an issue in
nsSecurityWarningDialogs.cpp, since the show_once stuff is only done by Firefox,
but ccing some Firefox folks.

DJC, if you get a chance to narrow down when (and whether) this regressed, that
would be most helpful.
Flags: blocking-aviary1.1?
(In reply to comment #4)

I have tested today for this bug on all the Firefox builds I happen to have on
my system (Linux) at the moment. They happen to be 20050126, 20050218 and
20050223. This bug occurs for me in all 3. The EXACT steps I followed to
reproduce are:

Firstly, save Testcase 1 to your computer.
Start Firefox with the parameters: -safe-mode and -profilemanager.
Create a brand new profile.
Select that profile and start Firefox.
Open the testcase from your computer. I used File -> Open File...
Click the button in that page.
As it's the first time you've used this profile, it should warn you about
submitting unencryped form data (show_once.)
Check the checkbox, and click OK.
When I do this, the warning appears again.

It then appears twice each time I test this bug, or nearly any other form. I
should be able to test on 3 Windows builds tomorrow, although I know I have
definitely witnessed it there.
OK, this is an issue for Firefox 1.0 as well....

That makes it sound like it's just never really worked right.
So.. in Firefox the nsSecureBrowserUIImpl is present twice in the form observer
list.  This happens because nsSecureBrowserUIImpl::Init is called twice on the
same nsSecureBrowserUIImpl object.  This should probably be asserting, since it
leads to incorrect behavior.  kaie, thoughts on that?

The first call comes from the "browser" binding constructor.  Presumably
is where that happens.  That's been in place since rev 1.1 of the file.

The second call comes from an onload handler.  To be precise, it comes from
prepareForStartup() in browser.js:

So looks like this has just never worked (both pieces of code have been in place
since fall 2002).

Over to Firefox.  Raising severity to critical, since this also adds the object
twice as a web progress listener, which may well confuse the lock icon code. 
There seems to be no "security" component, so I guess I'll dump this in general
and hope someone notices...
Assignee: dveditz → firefox
Severity: minor → critical
Component: Security: General → General
Ever confirmed: true
Product: Core → Firefox
QA Contact: general
> kaie, thoughts on that?

I have never used FireFox, I only use the suite.
If I understand correctly, the show_once is a FireFox specific feature, and this
bug only occurs with FireFox.

I agree an assertion is a good idea.

However, we could make the code more clever.

mWindow is only assigned a value in the init code.

If mWindow already carries a value, we could skip executing the init code again
(and produce an assertion only).
Attached patch protective patch v1 (obsolete) — Splinter Review
like this
Well, no reason to assert then.  Just throw NS_ERROR_ALREADY_INITIALIZED if
mWindow is already set.

Note that this patch breaks the firefox UI pretty badly (since browser.js ends
up with an exception and stuff doesn't get set up correctly).  So we'll have to
wait on the front-end to be fixed to check this in....
Attachment #175548 - Flags: superreview?(jst)
Attachment #175548 - Flags: review?(kaie)
Attachment #175547 - Attachment is obsolete: true
Comment on attachment 175548 [details] [diff] [review]
Patch for secure browser UI

Attachment #175548 - Flags: review?(kaie) → review+
Then we should probably have a separate firefox-frontend-change-request-bug and
make it block this one.
Comment on attachment 175548 [details] [diff] [review]
Patch for secure browser UI

Attachment #175548 - Flags: superreview?(jst) → superreview+
Apparently it *hasn't* ever worked right, found bug 180962 which has a patch to
comment out one of the inits.
Assignee: firefox → bzbarsky
Depends on: 180962
OK, I guess this is now the PSM bug on making it impossible to make this mistake

Daniel, could we try to get bug 180962 in for 1.8/1.1?
Component: General → Client Library
Flags: review+
Product: Firefox → PSM
Version: Trunk → unspecified
Priority: -- → P1
Summary: Warning appears twice when submitting unencrypted form data → [FIXr]Warning appears twice when submitting unencrypted form data
Product: PSM → Core
lets try to get this and bug 180962 for 1.1
Flags: blocking-aviary1.1? → blocking-aviary1.1+
Attachment #174747 - Attachment is obsolete: true
Per Asa's n.p.m.seamonkey post, this should probably go into b2 (being a bit of
an api change and all).
Flags: blocking1.8b2?
Flags: blocking1.8b2? → blocking1.8b2+
Comment on attachment 175548 [details] [diff] [review]
Patch for secure browser UI

Requesting approval for 1.8b2, now that bug 180962  is fixed.
Attachment #175548 - Flags: approval1.8b2?
Comment on attachment 175548 [details] [diff] [review]
Patch for secure browser UI

Attachment #175548 - Flags: approval1.8b2? → approval1.8b2+
Closed: 19 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.8beta2
Version: Other Branch → Trunk
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.