qtype is blindly added to the query without checking for validity

[Mike Rylander]

Evergreen version: 3.0 and newer

Currently, the UI-level qtype parameter is not checked for validity and is blindly added to the front of the constructed query string. In practice, that is being used in the wild to attempt SQL injections attacks. These attacks fail their intended purpose of proving a more direct attack is possible -- we don't execute user-provided strings as SQL -- but it can (and is, in some cases) cause an OOM condition due to the search query complexity. This is related to bug 1775958.

We should be checking the qtype param (and advanced search equivalents) with a regex, at the very least, to make sure it does not contain spaces or parens. We can't test for every possible qtype though, because the format for this is allowed to contain |s and an unordered set of field names after the class name or class alias.

[Mike Rylander (mrylander)]

I've pushed a branch to security/user/miker/lp-1811685-check-opac-qtype that addresses this by validating the qtype parameter against known search classes and aliases. From the commit message:

With this commit we throw away searches with invalid qtype value based on configured classes and aliases. Invalid qtype values have been seen in the wild as part of attempted (but failed) SQL injection attacks, so we will tighten up what we accept.

As an additional (unrelated) bonus, this commit also avoids prepending the search class on basic search when the class (from qytpe) is not exactly "keyword".

[Jason Stephenson (jstephenson)]

I have tested Mike's branch, and it works for me.

I've pushed a signed off branch to security/user/dyrcona/lp-1811685-check-opac-qtype-signoff

Thanks, Mike!

[Mike Rylander (mrylander)]

I've pushed a rebased branch with release notes to: security/user/miker/lp-1811685-check-opac-qtype-signoff-rebase

[Galen Charlton (gmc)]

Committed in the branches that will be used for building the March 2023 releases. Thanks, Mike and Jason!