piskvorky / sqlitedict

Persistent dict, backed by sqlite3 and pickle, multithread-safe.
Apache License 2.0
1.17k stars 131 forks source link

How can I report if there's a potential vulnerability #174

Closed William957-web closed 5 months ago

William957-web commented 6 months ago

I found out a vulnerability in this library, how can I report it? Already reported cve.

William957-web commented 6 months ago

@piskvorky

mpenkov commented 6 months ago

I think you can just report the issue here

William957-web commented 5 months ago

@mpenkov @piskvorky CVE-2024-35515

piskvorky commented 5 months ago

Thanks. Closing until there's a clearly demonstrated proof-of-concept or attack vector. Ideally with a mitigation PR where relevant.

William957-web commented 5 months ago

@piskvorky https://william957-web.github.io/sqlitedict-vuln-report.zip Additional details

mpenkov commented 5 months ago

Isn't this a problem with pickle, not with sqlitedict itself?

William957-web commented 5 months ago

@mpenkov Probably not, for example, you won't say that code injection vulnerbility is the problem with eval. In fact, there're serveral prevention due to pickle deserialization(like check object titles, sandboxes...), and ML often used libraries like clearML, pytorch also patched this kind of vulnerabilities.

William957-web commented 4 months ago

@mpenkov @piskvorky patched version (Reference:https://docs.python.org/3/library/pickle.html):

import builtins
import io
import pickle

safe_builtins = {
    'range',
    'complex',
    'set',
    'frozenset',
    'slice',
}

class RestrictedUnpickler(pickle.Unpickler):

    def find_class(self, module, name):
        # Only allow safe classes from builtins.
        if module == "builtins" and name in safe_builtins:
            return getattr(builtins, name)
        # Forbid everything else.
        raise pickle.UnpicklingError("global '%s.%s' is forbidden" %
                                     (module, name))

def restricted_loads(s):
    """Helper function analogous to pickle.loads()."""
    return RestrictedUnpickler(io.BytesIO(s)).load()

and change the decode and decode_key function's loads into restricted_loads. The only drawback of this prevention is that user can't store other none builtin data types(like numpy or else(but I think the functions can be replaced just by sqlitedict!))...

yoni13 commented 3 months ago

Just asking,is this vuln patched?

mpenkov commented 3 months ago

No, we didn't consider this worth patching, right @piskvorky ?

piskvorky commented 3 months ago

Correct.

yoni13 commented 3 months ago

After I read those pdfs, I think add a warning to warn user don't load untrusted db file is actually enough.

What do you think @William957-web ?

William957-web commented 3 months ago

@yoni13 Yeah, I agree with your idea! @piskvorky