Implementing mixed content for forms posting to insecure location from secure ones

Source/core/html/HTMLFormElement.cpp

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2001 Dirk Mueller (mueller@kde.org)
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009 Apple Inc. All rights reserved.
  *           (C) 2006 Alexey Proskuryakov (ap@nypop.com)
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
  * License as published by the Free Software Foundation; either
@@ skipping 19 matching lines @@
 #include "bindings/v8/ScriptController.h"
 #include "bindings/v8/ScriptEventListener.h"
 #include "core/dom/Attribute.h"
 #include "core/dom/Document.h"
 #include "core/dom/ElementTraversal.h"
 #include "core/dom/IdTargetObserverRegistry.h"
 #include "core/events/AutocompleteErrorEvent.h"
 #include "core/events/Event.h"
 #include "core/events/GenericEventQueue.h"
 #include "core/events/ScopedEventQueue.h"
+#include "core/frame/DOMWindow.h"
+#include "core/frame/LocalFrame.h"
+#include "core/frame/UseCounter.h"
+#include "core/frame/csp/ContentSecurityPolicy.h"

[abarth-chromium, 2014/06/09 22:06:23]

This header doesn't appear to be necessary.  Are all these new headers needed?

[mhm, 2014/06/09 23:27:59]

I didn't include them myself. I just moved them up to comply with the
alphabetical order of formatting pre-submit was complaining about.

 #include "core/html/HTMLCollection.h"
 #include "core/html/HTMLDialogElement.h"
 #include "core/html/HTMLImageElement.h"
 #include "core/html/HTMLInputElement.h"
 #include "core/html/HTMLObjectElement.h"
 #include "core/html/RadioNodeList.h"
 #include "core/html/forms/FormController.h"
 #include "core/loader/FrameLoader.h"
 #include "core/loader/FrameLoaderClient.h"
-#include "core/frame/DOMWindow.h"
-#include "core/frame/LocalFrame.h"
-#include "core/frame/UseCounter.h"
-#include "core/frame/csp/ContentSecurityPolicy.h"
+#include "core/loader/MixedContentChecker.h"
 #include "core/rendering/RenderTextControl.h"
 #include "platform/UserGestureIndicator.h"
+#include "wtf/text/AtomicString.h"
 
 using namespace std;
 
 namespace WebCore {
 
 using namespace HTMLNames;
 
 HTMLFormElement::HTMLFormElement(Document& document)
     : HTMLElement(formTag, document)
 #if !ENABLE(OILPAN)
@@ skipping 320 matching lines @@
     ASSERT(submission->state());
     if (submission->action().isEmpty())
         return;
     if (document().isSandboxed(SandboxForms)) {
         // FIXME: This message should be moved off the console once a solution to https://bugs.webkit.org/show_bug.cgi?id=103274 exists.
         document().addConsoleMessage(SecurityMessageSource, ErrorMessageLevel, "Blocked form submission to '" + submission->action().elidedString() + "' because the form's frame is sandboxed and the 'allow-forms' permission is not set.");
         return;
     }
 
     if (protocolIsJavaScript(submission->action())) {
-        if (!document().contentSecurityPolicy()->allowFormAction(KURL(submission->action())))
+        if (!document().contentSecurityPolicy()->allowFormAction(submission->action()))
             return;
         document().frame()->script().executeScriptIfJavaScriptURL(submission->action());
         return;
     }
 
     LocalFrame* targetFrame = document().frame()->loader().findFrameForNavigation(submission->target(), submission->state()->sourceDocument());
     if (!targetFrame) {
         if (!DOMWindow::allowPopUp(*document().frame()) && !UserGestureIndicator::processingUserGesture())
             return;
         targetFrame = document().frame();
     } else {
         submission->clearTarget();
     }
     if (!targetFrame->page())
         return;
 
+    if (MixedContentChecker::isMixedContent(document().securityOrigin(), submission->action())) {
+        UseCounter::count(document(), UseCounter::MixedContentFormsSubmitted);
+        if (!document().frame()->loader().mixedContentChecker()->canSubmitToInsecureForm(document().securityOrigin(), submission->action()))
+            return;
+    }
+
+    UseCounter::count(document(), UseCounter::FormsSubmitted);

[abarth-chromium, 2014/06/09 22:06:23]

Can we put this into the |else| branch of the previous if-statement?  Otherwise,
the stats will be skewed by the canSubmitToInsecureForm setting.

[mhm, 2014/06/09 23:27:59]

Done.

+
     submission->setReferrer(Referrer(document().outgoingReferrer(), document().referrerPolicy()));
     submission->setOrigin(document().outgoingOrigin());
 
     targetFrame->navigationScheduler().scheduleFormSubmission(submission);
 }
 
 void HTMLFormElement::reset()
 {
     LocalFrame* frame = document().frame();
     if (m_isInResetFunction || !frame)
@@ skipping 48 matching lines @@
     else
         ASSERT_NOT_REACHED();
 
     event->setTarget(this);
     m_pendingAutocompleteEventsQueue->enqueueEvent(event.release());
 }
 
 void HTMLFormElement::parseAttribute(const QualifiedName& name, const AtomicString& value)
 {
     if (name == actionAttr)
-        m_attributes.parseAction(value);
+        m_attributes.parseAction(document(), value);
     else if (name == targetAttr)
         m_attributes.setTarget(value);
     else if (name == methodAttr)
         m_attributes.updateMethodType(value);
     else if (name == enctypeAttr)
         m_attributes.updateEncodingType(value);
     else if (name == accept_charsetAttr)
         m_attributes.setAcceptCharset(value);
     else if (name == onautocompleteAttr)
         setAttributeEventListener(EventTypeNames::autocomplete, createAttributeEventListener(this, name, value));
@@ skipping 292 matching lines @@
     returnValue0 = radioNodeList(name, onlyMatchImg);
 }
 
 void HTMLFormElement::setDemoted(bool demoted)
 {
     if (demoted)
         UseCounter::count(document(), UseCounter::DemotedFormElement);
     m_wasDemoted = demoted;
 }
 
+void HTMLFormElement::attributeChanged(const QualifiedName& name, const AtomicString& newValue, AttributeModificationReason)
+{
+    Element::attributeChanged(name, newValue);
+    if (name == actionAttr) {
+        // If the new action attribute is pointing to insecure "action" location from a secure page
+        // it is marked as "passive" mixed content. In other words, it will just
+        // show a console warning unless the user override the preferences to
+        // block all mixed content.
+        KURL actionURL = (m_attributes.action().isEmpty() ? document().url() : m_attributes.action());
+        if (MixedContentChecker::isMixedContent(document().securityOrigin(), actionURL)) {
+            document().frame()->loader().mixedContentChecker()->canSubmitToInsecureForm(document().securityOrigin(), actionURL);
+        }
+    }
+}

[abarth-chromium, 2014/06/09 22:06:23]

Please delete this function.  We don't want to override this function.

[mhm, 2014/06/09 23:27:59]

The reason we have this here is that we want to give developer a console warning
of "passive" mixed content when they have a form with insecure action. This is
should during loading and when dynamically changing the attribute. The main goal
is to alert developers and drive them away from such insecure practices.

[mhm, 2014/06/10 00:40:57]

Done.

+
 } // namespace