Closed Bug 1287989 Opened 3 years ago Closed 3 years ago

Required empty inputs inside sandboxed iframe do validate

Categories

(Core :: DOM: Security, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla51
Tracking Status
firefox51 --- fixed

People

(Reporter: Oriol, Assigned: bzbarsky)

References

Details

Attachments

(1 file)

Run this code in a page or in a console:

    var iframe = document.createElement('iframe');
    iframe.sandbox = 'allow-scripts';
    iframe.src = 'data:text/html,'
      + '<form onsubmit="console.log(\'wrong\'); return false">'
        + '<input required oninvalid="console.log(\'good\'); return false" />'
        + '<input type="submit" />'
      + '</form>'
      + '<script>document.querySelector("input + input").click()<\/script>';
    document.body.appendChild(iframe);

A submit event is dispatched to the form, as if there weren't any invalid fields.

I would have expected an invalid event instead, as Chrome does.

Or, according to https://www.w3.org/TR/html5/forms.html#sandboxSubmitBlocked, no result at all.

Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=c676b554c7bb&tochange=1b51c7bf1e05

That is, when the sandbox attribute for IFRAMEs was implemented.
This is a great report, Oriol, thanks. Moving it to DOM->Security.
Component: Untriaged → DOM: Security
Hmm.  So we do this quite purposefully.  HTMLFormElement::CheckValidFormSubmission has:

  // Don't do validation for a form submit done by a sandboxed document that
  // doesn't have 'allow-forms', the submit will have been blocked and the
  // HTML5 spec says we shouldn't validate in this case.
  nsIDocument* doc = GetComposedDoc();
  if (doc && (doc->GetSandboxFlags() & SANDBOXED_FORMS)) {
    return true;
  }

Looking at the HTML spec, we're starting with a click on a submit input.  That lands at https://html.spec.whatwg.org/multipage/forms.html#submit-button-state-(type=submit) which has activation behavior that does https://html.spec.whatwg.org/multipage/forms.html#concept-form-submit

In that algorithm, step 2 aborts if the sandboxed forms flag is set.  That means there should be no "submit" event on the form, _and_ there should be no "invalid" event on the input.  As in, we should hoist the sandboxing check out of CheckValidFormSubmission and into a separate method, I think, called unconditionally.  Olli, you reviewed that code.  Do you recall why we implemented it this way, exactly?  Did the spec use to say something different?

I filed https://bugs.chromium.org/p/chromium/issues/detail?id=630628 on Chrome not matching the spec here.
Flags: needinfo?(bugs)
Yes, it seems IE/Edge is the only who follows the spec.

Another strange thing is that if you have an sandboxed (without allow-forms) iframe with

  <form>
    <input required />
    <input type="submit" />
  </form>

and click the submit button, the invalid input does not become red.

But if you enter some text, remove it back, and click the submit button again, now the input has a red box-shadow.
The red box-shadow is due to ui-invalid styling, which is unrelated to the form submission behavior and unaffacted by whether things are sandboxed, afaict.
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Flags: needinfo?(bugs)
Attachment #8776657 - Flags: review?(bugs) → review+
Pushed by bzbarsky@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/95ce26805472
Don't try to do validation or fire submit events in iframes that are sandboxed without allow-forms.  r=smaug
https://hg.mozilla.org/mozilla-central/rev/95ce26805472
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in before you can comment on or make changes to this bug.