Repository navigation
fix(security): escape SQL identifiers, validate order keys, quote the Pgsql DSN and check timezones - #136
Open
BenMacha wants to merge 1 commit into
Open
fix(security): escape SQL identifiers, validate order keys, quote the Pgsql DSN and check timezones#136BenMacha wants to merge 1 commit into
BenMacha wants to merge 1 commit into
Conversation
… Pgsql DSN and check timezones - escapeField() doubles backticks (MySQL) and double quotes (PostgreSQL); SphinxQL only accepts plain identifiers - Repository::getBy() order keys (entity property or column) and direction are checked against the metadata; Repository::get() refuses an unknown primary key - setTimezone() refuses a value that is not a timezone name; Pgsql connect() quotes every DSN value - ConnectionPool connection key no longer contains the clear-text password - Mysqli::execute() quotes inlined strings with apostrophes (safe with sql_mode=ANSI_QUOTES) - Pgsql::getInsertedId() throws when lastval() fails
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ValueException/InvalidArgumentExceptioninstead of producing invalid or injectable SQL)Why
While reviewing the drivers after #120 I found other places where a value reaches the SQL text without protection.
What
escapeField(): MySQL doubles backticks, PostgreSQL doubles double quotes, SphinxQL (which has no quoting) only accepts plain identifiers. Before, a column name containing the delimiter broke out of it.Repository::getBy($criteria, $forceMaster, $order): order keys were passed toORDER BYwithout being checked against the metadata (criteria keys are). They are now resolved like criteria (entity property or column name), an unknown key or a direction other thanASC/DESCthrows.LIMITis cast to int.Repository::get(array $primaries): keys are resolved to columns and must be declared primaries.setTimezone()(both drivers): the value is interpolated withsprintf; it is now validated against a timezone-name pattern.Pgsql\Driver::connect(): every DSN value is quoted, so a password containing a space or a quote no longer breaks the DSN nor injects an option. The redacted DSN in the error message follows.getConnectionKey(): the password is hashed (sha256) instead of being kept in clear text in the pool's array key.This key is only used as the index of
ConnectionPool::$connectionsto reuse an open connection; it is never stored, exposed or compared to a known string. Two different passwords still give two different keys and the same password the same key, so the pool behaves exactly as before. Signature andDriverInterfaceare unchanged: no BC break, nothing to change for users. The gain: avar_dump/ profiler / Sentry dump of the pool no longer shows the database password.Mysqli\Driver::quoteValue(): strings are quoted with apostrophes; withsql_mode=ANSI_QUOTESdouble quotes denote an identifier.Pgsql\Driver::getInsertedId(): throws aQueryExceptionwhenlastval()fails instead of callingpg_fetch_row(false).Tests adapted (DSN expectation, apostrophes, order key on a known column) and added (escaping of a delimiter, rejection of an unknown order column and of an invalid direction).
CHANGELOGgets anUnreleasedsection.