CVE-2023-34457 (Python, published 2023-07-05T20:15Z)
Description: value: MechanicalSoup is a Python library for automating interaction with websites. Starting in version 0.2.0 and prior to version 1.3.0, a malicious web server can read arbitrary files on the client using a `<input type="file" ...>` inside HTML form. All users of MechanicalSoup's form submission are affected, unless they took very specific (and manual) steps to reset HTML form field values. Version 1.3.0 contains a patch for this issue.
Severity: nan (CVSS v3 base score: 7.5)
Weakness: CWE-20 \u2014 Improper Input Validation
The product receives input or data, but it does
        not validate or incorrectly validates that the input has the
        properties that are required to process the data safely and
        correctly.
Repository: https://github.com/MechanicalSoup/MechanicalSoup
Files changed:
mechanicalsoup/browser.py
mechanicalsoup/form.py
mechanicalsoup/utils.py
tests/test_browser.py
tests/test_stateful_browser.py
Commit message: Merge pull request from GHSA-x456-3ccm-m6j4

* Add suggested mitigation against malicious HTML form file input. Test cases and docs not updated.

* Make tests pass. Add test specifically for this vulnerability. Fix the `Form.new_control()` method so that it correctly sets the input value if the value is a file object.

* Add better documentation about the vulnerability remediation. Wording fix.

* tests: Add separate test to check for CVE-2023-34457.

* Revert "tests: Add separate test to check for CVE-2023-34457."

This reverts commit bd4c6d92a8a803499a447b511bc46b1ba00841d0.

* Misc cleanup

* No need to raise the error on forms with an enctype that do not
  upload files (e.g. everything besides multipart).

* Make sure that open file inputs are converted to their basename
  regardless of what kind of form it is to avoid leaking local file
  paths.

* Throw error at time when the file input is set incorrectly, rather
  than at submission time.

* Factor complex repeated logic into their own functions (i.e. for
  the multipart file input check and invalid file value check).

* Fix failing tests.

* Avoid excessive whitespace in exception message.

* Reorder 'and' operands so the slightly less expensive check comes first.

---------

Co-authored-by: Dan Hemberger <daniel.hemberger@gmail.com>

Vulnerable code:
from .utils import LinkNotFoundError
                if tag.get("type", "").lower() == "file" and multipart:
                    filepath = value
                    if filepath != "" and isinstance(filepath, str):
                        content = open(filepath, "rb")
                    filename = os.path.basename(filepath)
                    # If value is the empty string, we still pass it
from .utils import LinkNotFoundError
        name="tagname">`` field (provide the path to the local file,
            form.set("tagname", path_to_local_file)
            expected_content = pic_path.encode()
        form.find("input", {"name": "pic"})["value"] = pic_path
    browser.open(httpbin + "/forms/post")
    path1, path2 = (make_file(content) for content in
                    ("first file content", "second file content"))
    # The form doesn't have a type=file field, but the target action
    # does show it => add the fields ourselves, and add enctype too.
    browser._StatefulBrowser__state.form.form[
      "enctype"] = "multipart/form-data"
    browser.new_control("file", "first", path1)
    browser.new_control("file", "second", "")
    browser["second"] = path2
    browser.form.print_summary()

Fixed code:
import io
from .utils import LinkNotFoundError, is_multipart_file_upload
                if is_multipart_file_upload(form, tag):
                    if isinstance(value, io.IOBase):
                        content = value
                        filename = os.path.basename(getattr(value, "name", ""))
                        filename = os.path.basename(value)
                    # If content is the empty string, we still pass it
                    if isinstance(value, io.IOBase):
                        value = os.path.basename(getattr(value, "name", ""))
import io
from .utils import LinkNotFoundError, is_multipart_file_upload
            self._assert_valid_file_upload(i, value)
        name="tagname">`` field (provide an open file object,
            form.set("tagname", open(path_to_local_file, "rb"))
        self._assert_valid_file_upload(control, value)

    def _assert_valid_file_upload(self, tag, value):
        """Raise an exception if a multipart file input is not an open file."""
        if (
            is_multipart_file_upload(self.form, tag) and
            not isinstance(value, io.IOBase)
        ):
            raise ValueError(
                "From v1.3.0 onwards, you must pass an open file object "
                'directly, e.g. `form["name"] = open("/path/to/file", "rb")`. '
                "This change is to remediate a security vulnerability where "
                "a malicious web server could read arbitrary files from the "
                "client (CVE-2023-34457)."
            )


def is_multipart_file_upload(form, tag):
    return (
        form.get("enctype", "") == "multipart/form-data" and
        tag.get("type", "").lower() == "file"
    )
            expected_content = os.path.basename(pic_path.encode())
        tag = form.find("input", {"name": "pic"})
        tag["value"] = open(pic_path, "rb")
    url = httpbin + "/post"
    file_input_form = f"""
    <form method="post" action="{url}" enctype="multipart/form-data">
        <input type="file" name="first" />
    </form>
    """
    path1 = make_file("first file content")
    path2 = make_file("second file content")
    value1 = open(path1, "rb")
    value2 = open(path2, "rb")

    browser.open_fake_page(file_input_form)

    # Test filling an existing input and creating a new input
    browser["first"] = value1
    browser.new_control("file", "second", value2)

def test_upload_file_with_malicious_default(httpbin):
    """Check for CVE-2023-34457 by setting the form input value directly to a
    file that the user does not explicitly consent to upload, as a malicious
    server might do.
    """
    browser = mechanicalsoup.StatefulBrowser()
    sensitive_path = tempfile.mkstemp()[1]
    with open(sensitive_path, "w") as fd:
        fd.write("Some sensitive information")
    url = httpbin + "/post"
    malicious_html = f"""
    <form method="post" action="{url}" enctype="multipart/form-data">
        <input type="file" name="malicious" value="{sensitive_path}" />
    </form>
    """
    browser.open_fake_page(malicious_html)
    browser.select_form()
    response = browser.submit_selected()
    assert response.json()["files"] == {"malicious": ""}


def test_upload_file_raise_on_string_input():
    """Check for use of the file upload API that was modified to remediate
    CVE-2023-34457. Users must now open files manually to upload them.
    """
    browser = mechanicalsoup.StatefulBrowser()
    file_input_form = """
    <form enctype="multipart/form-data">
        <input type="file" name="upload" />
    </form>
    """
    browser.open_fake_page(file_input_form)
    browser.select_form()
    with pytest.raises(ValueError, match="CVE-2023-34457"):
        browser["upload"] = "/path/to/file"
    with pytest.raises(ValueError, match="CVE-2023-34457"):
        browser.new_control("file", "upload2", "/path/to/file")