Merge pull request #1044 from Nothing4You/local-infile

Fix arbitrary file access vulnerability when connecting to malicious servers

Author: Nothing4You

CHANGES.txt

@@ -19,6 +19,11 @@ next (unreleased)
 * | Bump setuptools to >=80, setuptools-scm to  >=7, <10.
   | setuptools-scm must be at least 9.2.0 for consistent hash lengths of non-release builds.
 
+* | Properly check whether loading of local files is enabled #1044
+  | Loading local data now requires using the `local_infile` parameter, passing just the client flag through `client_flag` is no longer supported.
+  | Fixes `GHSA-r397-ff8c-wv2g <https://github.com/aio-libs/aiomysql/security/advisories/GHSA-r397-ff8c-wv2g>`_
+  | Thanks to @KonstantAnxiety for reporting this.
+
 0.2.0 (2023-06-11)
 ^^^^^^^^^^^^^^^^^^
 

aiomysql/connection.py

@@ -245,7 +245,8 @@ def __init__(self, host="localhost", user=None, password="",
 
         self._encoding = charset_by_name(self._charset).encoding
 
-        if local_infile:
+        self._local_infile = bool(local_infile)
+        if self._local_infile:
             client_flag |= CLIENT.LOCAL_FILES
 
         client_flag |= CLIENT.CAPABILITIES
@@ -1203,6 +1204,10 @@ def _read_ok_packet(self, first_packet):
         self.has_next = ok_packet.has_next
 
     async def _read_load_local_packet(self, first_packet):
+        if not self.connection._local_infile:
+            raise RuntimeError(
+                "**WARN**: Received LOAD_LOCAL packet but local_infile option is false."
+            )
         load_packet = LoadLocalPacketWrapper(first_packet)
         sender = LoadLocalFile(load_packet.filename, self.connection)
         try:

docs/connection.rst

@@ -46,8 +46,8 @@ Example::
             client_flag=0, cursorclass=Cursor, init_command=None,
             connect_timeout=None, read_default_group=None,
             autocommit=False, echo=False
-            ssl=None, auth_plugin='', program_name='',
-            server_public_key=None, loop=None)
+            local_infile=False, loop=None, ssl=None, auth_plugin='',
+            program_name='', server_public_key=None)
 
     A :ref:`coroutine <coroutine>` that connects to MySQL.
 
@@ -71,7 +71,8 @@ Example::
         See `pymysql.converters`.
     :param use_unicode: whether or not to default to unicode strings.
     :param  client_flag: custom flags to send to MySQL. Find
-        potential values in `pymysql.constants.CLIENT`.
+        potential values in `pymysql.constants.CLIENT`. Refer to the
+        `local_infile` parameter for enabling loading of local data.
     :param cursorclass: custom cursor class to use.
     :param str init_command: initial SQL statement to run when connection is
         established.
@@ -81,6 +82,10 @@ Example::
         file.
     :param autocommit: Autocommit mode. None means use server default.
         (default: ``False``)
+    :param local_infile: Boolean to enable the use of `LOAD DATA LOCAL`
+        command. This also enables the corresponding `client_flag`. aiomysql
+        does not perform any validation of files requested by the server. Do
+        not use this with untrusted servers. (default: ``False``)
     :param ssl: Optional SSL Context to force SSL
     :param auth_plugin: String to manually specify the authentication
         plugin to use, i.e you will want to use mysql_clear_password

tests/test_load_local.py

@@ -2,7 +2,9 @@
 import os
 from unittest.mock import patch, MagicMock
 
+import aiomysql
 import pytest
+from pymysql.constants import CLIENT
 from pymysql.err import OperationalError
 
 
@@ -81,3 +83,33 @@ async def test_load_warnings(cursor, table_local_file):
     with warnings.catch_warnings(record=True) as w:
         await cursor.execute(sql)
     assert "Incorrect integer value" in str(w[-1].message)
+
+
+@pytest.mark.run_loop
+async def test_load_local_disabled(mysql_params, table_local_file):
+    # By setting the client flag, the server will be informed that we support
+    # loading local files. This validates that the client side check catches
+    # the server attempting to read files from us without having this
+    # explicitly enabled on the connection. The local_infile parameter sets
+    # the client flag, but not the other way round.
+    params = mysql_params.copy()
+    params["local_infile"] = False
+    if "client_flag" in params:
+        params["client_flag"] |= CLIENT.LOCAL_FILES
+    else:
+        params["client_flag"] = CLIENT.LOCAL_FILES
+
+    async with aiomysql.connect(**params) as conn:
+        async with conn.cursor() as cursor:
+            # Test load local infile with a valid file
+            filename = os.path.join(os.path.dirname(os.path.realpath(__file__)),
+                                    'fixtures',
+                                    'load_local_data.txt')
+            with pytest.raises(
+                RuntimeError,
+                match="Received LOAD_LOCAL packet but local_infile option is false",
+            ):
+                await cursor.execute(
+                    ("LOAD DATA LOCAL INFILE '{0}' INTO TABLE " +
+                     "test_load_local FIELDS TERMINATED BY ','").format(filename)
+                )