Possible SQL injection vulnerability

[Tenchberry]

Hi there,

I work at a contract research organisation (CRO) that has recently been interested in implementing mmpdb as a part of a drug discovery pipeline. A step of this implementation involves checking for potential security issues before it can be installed internally.

This check was failed due to two areas in the code where SQL injection vulnerabilities appear. For context, an SQL injection vulnerability is a technique in which a call normally used to execute SQL queries could be used by a malicious user to execute unintended actions, like the exposure of sensitive/confidential information in databases or the installation of malware etc.

Out IT team flagged two areas of the code where the vulnerabilities appear. Here is the first (in mmpdblib/peewee.py):-

   def execute_sql(self, sql, params=None, require_commit=True):
        logger.debug((sql, params))
        with self.exception_wrapper():
            cursor = self.get_cursor()
            try:
                cursor.execute(sql, params or ())
            except Exception as exc:
                if self.get_autocommit() and self.autorollback:
                    self.rollback()
                if self.sql_error_handler(exc, sql, params, require_commit):
                    raise
            else:
                if require_commit and self.get_autocommit():
                    self.commit()
        return cursor

The second is in the same location, so I assume their method of flagging vulnerabilities has picked this same SQL injection problem twice.

Would you be interested in addressing this particular vulnerability? If not, this isn't an immediate problem as we can address it internally and submit a PR with the fix? Let me know how you'd like to proceed with this, if at all

Thanks

[adalke]

I don't see a SQL injection vulnerability. Everything seems to be done correctly.

For what it's worth, this is a vendored version of the peewee package from https://github.com/coleifer/peewee .

[Tenchberry]

Hi there. So we're likely going to deal with this internally as we're not sure if the point of this being a vulnerability is completely fair. We'll keep in touch but for now I think this issue can be closed