CVE-2023-47128 (Python, published 2023-11-10T18:15Z)
Description: value: Piccolo is an object-relational mapping and query builder which supports asyncio. Prior to version 1.1.1, the handling of named transaction `savepoints` in all database implementations is vulnerable to SQL Injection via f-strings. While the likelihood of an end developer exposing a `savepoints` `name` parameter to a user is highly unlikely, it would not be unheard of. If a malicious user was able to abuse this functionality they would have essentially direct access to the database and the ability to modify data to the level of permissions associated with the database user. A non exhaustive list of actions possible based on database permissions is: Read all data stored in the database, including usernames and password hashes; insert arbitrary data into the database, including modifying existing records; and gain a shell on the underlying server. Version 1.1.1 fixes this issue.
Severity: nan (CVSS v3 base score: 9.1)
Weakness: CWE-89 \u2014 Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection')
The product constructs all or part of an SQL command using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify the intended SQL command when it is sent to a downstream component. Without sufficient removal or quoting of SQL syntax in user-controllable inputs, the generated SQL query can cause those inputs to be interpreted as SQL instead of ordinary user data.
Repository: https://github.com/piccolo-orm/piccolo
Files changed:
piccolo/engine/base.py
piccolo/engine/postgres.py
piccolo/engine/sqlite.py
tests/engine/test_transaction.py
Commit message: Merge pull request from GHSA-xq59-7jf3-rjc6

Co-authored-by: skelmis <ethan.mckee-harris@zxsecurity.co.nz>

Vulnerable code:
from piccolo.engine.base import Batch, Engine
from piccolo.engine.base import Batch, Engine

Fixed code:
import string
# This is a set to speed up lookups from O(n) when
# using str vs O(1) when using set[str]
VALID_SAVEPOINT_CHARACTERS: t.Final[set[str]] = set(
    string.ascii_letters + string.digits + "-" + "_"
)


def validate_savepoint_name(savepoint_name: str) -> None:
    """Validates a save point's name meets the required character set."""
    if not all(i in VALID_SAVEPOINT_CHARACTERS for i in savepoint_name):
        raise ValueError(
            "Savepoint names can only contain the following characters:"
            f" {VALID_SAVEPOINT_CHARACTERS}"
        )
from piccolo.engine.base import Batch, Engine, validate_savepoint_name
        validate_savepoint_name(self.name)
        validate_savepoint_name(self.name)
        validate_savepoint_name(name)
from piccolo.engine.base import Batch, Engine, validate_savepoint_name
        validate_savepoint_name(self.name)
        validate_savepoint_name(self.name)
        validate_savepoint_name(name)
import pytest


    def test_savepoint_sqli_checks(self):
        # Added to test the fix for GHSA-xq59-7jf3-rjc6
        async def run_test():
            async with Manager._meta.db.transaction() as transaction:
                await transaction.savepoint(
                    "my_savepoint; SELECT * FROM Manager"
                )

        with pytest.raises(ValueError):
            run_sync(run_test())