Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

sqlite3.Connection(...) bypasses 'sqlite3.connect' audit hooks #87600

Closed
erlend-aasland opened this issue Mar 8, 2021 · 13 comments
Closed

sqlite3.Connection(...) bypasses 'sqlite3.connect' audit hooks #87600

erlend-aasland opened this issue Mar 8, 2021 · 13 comments
Labels
3.8 only security fixes 3.9 only security fixes 3.10 only security fixes stdlib Python modules in the Lib dir type-security A security issue

Comments

@erlend-aasland
Copy link
Contributor

BPO 43434
Nosy @berkerpeksag, @zooba, @miss-islington, @erlend-aasland
PRs
  • bpo-43434: Move sqlite3.connect audit events to sqlite3.Connection.__init__ #25818
  • [3.9] bpo-43434: Move sqlite3.connect audit event to sqlite3.Connection.__init__ (GH-25818) #25822
  • bpo-43434: Clean up after GH-25818 #25823
  • [3.8] bpo-43434: Move sqlite3.connect audit event to sqlite3.Connection.__init__ (GH-25818) #25825
  • [3.8] bpo-43434: Move sqlite3.connect audit event to sqlite3.Connection.__init__ (GH-25818) #25826
    		•
    Files
  • audit.py
  • patch.diff
    		•

    Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
closed_at = <Date 2021-05-02.22:56:58.153>
created_at = <Date 2021-03-08.12:53:59.903>
labels = ['type-security', '3.8', 'library', '3.9', '3.10']
title = "sqlite3.Connection(...) bypasses 'sqlite3.connect' audit hooks"
updated_at = <Date 2021-05-02.22:56:58.152>
user = 'https://github.com/erlend-aasland'

    bugs.python.org fields:

    activity = <Date 2021-05-02.22:56:58.152>
actor = 'steve.dower'
assignee = 'none'
closed = True
closed_date = <Date 2021-05-02.22:56:58.153>
closer = 'steve.dower'
components = ['Library (Lib)']
creation = <Date 2021-03-08.12:53:59.903>
creator = 'erlendaasland'
dependencies = []
files = ['49857', '49858']
hgrepos = []
issue_num = 43434
keywords = ['patch']
message_count = 13.0
messages = ['388264', '392393', '392702', '392709', '392724', '392735', '392736', '392737', '392742', '392743', '392745', '392748', '392749']
nosy_count = 4.0
nosy_names = ['berker.peksag', 'steve.dower', 'miss-islington', 'erlendaasland']
pr_nums = ['25818', '25822', '25823', '25825', '25826']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'security'
url = 'https://bugs.python.org/issue43434'
versions = ['Python 3.8', 'Python 3.9', 'Python 3.10']
    @erlend-aasland
    Copy link
    Contributor Author

    The module level connect method is guarded by PySys_Audit(), but sqlite3.Connection.__init__() is not. It is possible to bypass the module level connect() method simply by creating a new sqlite3.Connection object directly.

    Easily fixed by either moving the PySys_Audit() check to pysqlite_connection_init(), or by adding an extra check in pysqlite_connection_init().

    >>> import sqlite3, sys
>>> def hook(s, e):
...     if s == 'sqlite3.connect':
...             raise PermissionError
... 
>>> sys.addaudithook(hook)
>>> sqlite3.connect(':memory:')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<stdin>", line 3, in hook
PermissionError
>>> sqlite3.Connection(':memory:')
<sqlite3.Connection object at 0x7f94b0157a80>

    @erlend-aasland erlend-aasland added 3.8 only security fixes 3.9 only security fixes 3.10 only security fixes stdlib Python modules in the Lib dir type-security A security issue labels Mar 8, 2021
    @erlend-aasland
    Copy link
    Contributor Author

    Steve, is it worth it to improve this?

    @zooba
    Copy link
    Member

    zooba commented May 2, 2021

    Yes, let's move it into the init function.

    @erlend-aasland
    Copy link
    Contributor Author

    The PR accidentally fixes a bug in #69433 (bpo-43762):
    The object passed to PySys_Audit() is now the connection object, not the module object.

    @erlend-aasland
    Copy link
    Contributor Author

    Steve, can we get this in before beta1 (bco. the bugfix)?

    @zooba
    Copy link
    Member

    zooba commented May 2, 2021

    New changeset c96cc08 by Erlend Egeberg Aasland in branch 'master':
    bpo-43434: Move sqlite3.connect audit events to sqlite3.Connection.__init__ (GH-25818)
    c96cc08

    @zooba
    Copy link
    Member

    zooba commented May 2, 2021

    We could get this one in after beta 1 anyway, but sure, it's in.

    The backports are going to have to be manual, I suspect...

    @erlend-aasland
    Copy link
    Contributor Author

    Thanks! :) I'll fix the backports.

    @zooba
    Copy link
    Member

    zooba commented May 2, 2021

    Thanks, Erlend! Appreciate how quickly you got onto that, and the quality of your work.

    @zooba
    Copy link
    Member

    zooba commented May 2, 2021

    New changeset cbb7b9e by Erlend Egeberg Aasland in branch 'master':
    bpo-43434: Clean up sqlite3.connect() after #70004 (GH-25823)
    cbb7b9e

    @erlend-aasland
    Copy link
    Contributor Author

    Thanks, Steve, that means a lot! Glad to help. Thank you for getting it into beta1. Having the new event out there with the wrong object passed to it would have been a tiny bit embarrassing :)

    @zooba
    Copy link
    Member

    zooba commented May 2, 2021

    New changeset ad73d16 by Erlend Egeberg Aasland in branch '3.9':
    bpo-43434: Move sqlite3.connect audit event to sqlite3.Connection.__init__ (GH-25818)
    ad73d16

    @zooba
    Copy link
    Member

    zooba commented May 2, 2021

    New changeset 10665ac by Erlend Egeberg Aasland in branch '3.8':
    bpo-43434: Move sqlite3.connect audit events to sqlite3.Connection.__init__ (GH-25818)
    10665ac

    @zooba zooba closed this as completed May 2, 2021
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    3.8 only security fixes 3.9 only security fixes 3.10 only security fixes stdlib Python modules in the Lib dir type-security A security issue
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    2 participants
    @zooba @erlend-aasland