Changeset 20074 in webkit

Timestamp:

Mar 8, 2007 6:25:25 PM (17 years ago)

Author:

darin

Message:

Reviewed by Adele.

• <rdar://problem/4470381> multipart/form-data boundary security vulnerability

By making the form data boundary a string with some random data in it, we reduce
the possibility that anyone could take advantage of it by creating a file that
intentionally has the boundary string in it.

• html/HTMLFormElement.h: Removed boundary(), setBoundary(), and m_boundary. Marked a lot more stuff private.
• html/HTMLFormElement.cpp: (WebCore::HTMLFormElement::HTMLFormElement): Removed code to initialize m_boundary. (WebCore::randomNumber): Added. Function that returns a random number, including seeding the random number generator the first time it's called. For now, usees the more random function random() on Mac OS X and the more-standard rand() on other platforms. (WebCore::HTMLFormElement::formData): Take a parameter with the form boundary string, and use that instead of m_boundary. (WebCore::getUniqueBoundaryString): Added. Makes a boundary string using random numbers and base 64 encoding. (WebCore::HTMLFormElement::submit): Call getUniqueBoundaryString and pass the boundary string into formData for multipart form posts.

Location:

trunk/WebCore

Files:

• ChangeLog (modified) (1 diff)
• html/HTMLFormElement.cpp (modified) (9 diffs)
• html/HTMLFormElement.h (modified) (4 diffs)

trunk/WebCore/ChangeLog

@@ +1 @@
+2007-03-08  Darin Adler  <darin@apple.com>
+
+        Reviewed by Adele.
+
+        - <rdar://problem/4470381> multipart/form-data boundary security vulnerability
+
+        By making the form data boundary a string with some random data in it, we reduce
+        the possibility that anyone could take advantage of it by creating a file that
+        intentionally has the boundary string in it.
+
+        * html/HTMLFormElement.h: Removed boundary(), setBoundary(), and m_boundary.
+        Marked a lot more stuff private.
+        * html/HTMLFormElement.cpp:
+        (WebCore::HTMLFormElement::HTMLFormElement): Removed code to initialize
+        m_boundary.
+        (WebCore::randomNumber): Added. Function that returns a random number, including
+        seeding the random number generator the first time it's called. For now, usees the more
+        random function random() on Mac OS X and the more-standard rand() on other platforms.
+        (WebCore::HTMLFormElement::formData): Take a parameter with the form boundary string,
+        and use that instead of m_boundary.
+        (WebCore::getUniqueBoundaryString): Added. Makes a boundary string using random numbers
+        and base 64 encoding.
+        (WebCore::HTMLFormElement::submit): Call getUniqueBoundaryString and pass the boundary
+        string into formData for multipart form posts.
+
 2007-03-08  Maciej Stachowiak  <mjs@apple.com>
 

trunk/WebCore/html/HTMLFormElement.cpp

@@ -1 +1 @@
 /*
- * This file is part of the DOM implementation for KDE.
- *
  * 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 Apple Computer, Inc.
+ * Copyright (C) 2004, 2005, 2006, 2007 Apple Inc. All rights reserved.
  *           (C) 2006 Alexey Proskuryakov (ap@nypop.com)
  *
@@ -28 +26 @@
 #include "HTMLFormElement.h"
 
+#include "Base64.h"
 #include "CString.h"
 #include "csshelper.h"
@@ -60 +59 @@
     m_inreset = false;
     m_enctype = "application/x-www-form-urlencoded";
-    m_boundary = "----------0xKhTmLbOuNdArY";
     m_malformed = false;
     m_preserveAcrossRemove = false;
@@ -197 +195 @@
 }
 
-PassRefPtr<FormData> HTMLFormElement::formData() const
-{
-    DeprecatedCString enc_string = ""; // used for non-multipart data
+static int randomNumber()
+{
+    static bool randomSeeded = false;
+
+#if PLATFORM(DARWIN)
+    if (!randomSeeded) {
+        srandomdev();
+        randomSeeded = true;
+    }
+    return random();
+#else
+    if (!randomSeeded) {
+        srand(static_cast<unsigned>(time(0)));
+        randomSeeded = true;
+    }
+    return rand();
+#endif
+}
+
+PassRefPtr<FormData> HTMLFormElement::formData(const char* boundary) const
+{
+    DeprecatedCString enc_string = "";
 
     String str = m_acceptcharset;
@@ -246 +263 @@
                 {
                     DeprecatedCString hstr("--");
-                    hstr += m_boundary.deprecatedString().latin1();
+                    hstr += boundary;
                     hstr += "\r\n";
                     hstr += "Content-Disposition: form-data; name=\"";
@@ -295 +312 @@
 
 
-    if (m_multipart)
-        enc_string = ("--" + m_boundary.deprecatedString() + "--\r\n").ascii();
+    if (m_multipart) {
+        enc_string = "--";
+        enc_string += boundary;
+        enc_string += "--\r\n";
+    }
 
     result->appendData(enc_string.data(), enc_string.length());
@@ -316 +336 @@
 }
 
-void HTMLFormElement::setBoundary( const String& bound )
-{
-    m_boundary = bound;
-}
-
 bool HTMLFormElement::prepareSubmit(Event* event)
 {
@@ -344 +359 @@
 {
     submit(0, false);
+}
+
+// Returns a 0-terminated C string in the vector.
+static void getUniqueBoundaryString(Vector<char>& boundary)
+{
+    // Start with an informative prefix.
+    const char boundaryPrefix[] = "----WebKit-form-boundary-";
+    boundary.append(boundaryPrefix, strlen(boundaryPrefix));
+
+    // Append 16 characters of base 64 randomness.
+    Vector<char> randomBytes;
+    for (int i = 0; i < 3; ++i) {
+        int randomness = randomNumber();
+        randomBytes.append(static_cast<char>(randomness >> 24));
+        randomBytes.append(static_cast<char>(randomness >> 16));
+        randomBytes.append(static_cast<char>(randomness >> 8));
+        randomBytes.append(static_cast<char>(randomness));
+    }
+    Vector<char> randomBase64;
+    base64Encode(randomBytes, randomBase64);
+    boundary.append(randomBase64);
+
+    // Add a 0 at the end so we can use this as a C-style string.
+    boundary.append(0);
 }
 
@@ -386 +425 @@
 
     if (!m_post)
+
+    if (m_post) {
+        if (!m_multipart)
+            frame->loader()->submitForm("POST", m_url, formData(0), m_target, enctype(), String(), event);
+        else {
+            Vector<char> boundary;
+            getUniqueBoundaryString(boundary);
+            frame->loader()->submitForm("POST", m_url, formData(boundary.data()), m_target, enctype(), boundary.data(), event);
+        }
+    } else {
         m_multipart = false;
-    
-    if (m_post)
-        frame->loader()->submitForm("POST", m_url, formData(), m_target, enctype(), boundary(), event);
-    else
-        frame->loader()->submitForm("GET", m_url, formData(), m_target, String(), String(), event);
+        frame->loader()->submitForm("GET", m_url, formData(0), m_target, String(), String(), event);
+    }
 
     if (needButtonActivation && firstSuccessfulSubmitButton)

trunk/WebCore/html/HTMLFormElement.h

@@ -1 +1 @@
 /*
- * This file is part of the DOM implementation for KDE.
- *
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2000 Dirk Mueller (mueller@kde.org)
- * Copyright (C) 2004, 2005, 2006 Apple Computer, Inc.
+ * Copyright (C) 2004, 2005, 2006, 2007 Apple Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
@@ -62 +60 @@
     void setEncoding(const String& enctype) { setEnctype(enctype); }
 
-    String boundary() const { return m_boundary; }
-    void setBoundary(const String&);
-
     bool autoComplete() const { return m_autocomplete; }
 
@@ -105 +100 @@
     void setTarget(const String&);
 
+    // FIXME: Change this to be private after getting rid of all the clients.
+    Vector<HTMLGenericFormElement*> formElements;
+
+private:
+    void parseEnctype(const String&);
+    PassRefPtr<FormData> formData(const char* boundary) const;
+    unsigned formElementIndex(HTMLGenericFormElement*);
+
     friend class HTMLFormCollection;
 
     HTMLCollection::CollectionInfo* collectionInfo;
 
-    Vector<HTMLGenericFormElement*> formElements;
     Vector<HTMLImageElement*> imgElements;
     String m_url;
@@ -125 +127 @@
     bool m_preserveAcrossRemove : 1;
 
-private:
-    void parseEnctype(const String&);
-    PassRefPtr<FormData> formData() const;
-
-    unsigned formElementIndex(HTMLGenericFormElement*);
-
     String oldNameAttr;
 };