Skip to content

Fix SQL injection in biobank DAO selectInstances and cascade update#10676

Open
HenriRabalais wants to merge 1 commit into
aces:28.0-releasefrom
HenriRabalais:2026-06-22_biobank-sql-injection-parameterize
Open

Fix SQL injection in biobank DAO selectInstances and cascade update#10676
HenriRabalais wants to merge 1 commit into
aces:28.0-releasefrom
HenriRabalais:2026-06-22_biobank-sql-injection-parameterize

Conversation

@HenriRabalais

Copy link
Copy Markdown
Collaborator

Brief summary of changes

Fixes SQL injection in the biobank module's DAO layer. The selectInstances
methods in the container, specimen, pool, and shipment DAOs built their WHERE
clauses by concatenating $condition['value'] directly into the query string
and passed an empty parameter array to pselectWithIndexKey. User-supplied
values reaching these methods (e.g. a container barcode or shipmentBarcodes
via the container endpoint) could break out of the quoted string and inject
arbitrary SQL.

Both reported exploits hit the same DAO code through different fields:

  • barcode -> containerdao::selectInstances (via container validation)
  • shipmentBarcodes -> shipmentdao::selectInstances (via shipment validation)

All four selectInstances methods now bind values to named placeholders and
pass them through to pselectWithIndexKey. Column names are left interpolated
as before, since every caller supplies them as hardcoded literals, never from
user input.

Additionally, containerdao::_cascadeToChildren built an UPDATE ... SET $field=$value query with the value interpolated raw. $value originates from
user-submitted container data (temperature, status, center), so this was a
second injection point on the same endpoint. It now binds the value via a
prepared statement.

Testing instructions

  1. Check out this branch on a LORIS instance with the biobank module enabled.
  2. Log in and obtain a session cookie.
  3. Attempt the reported exploit against the container endpoint, e.g. a POST with
    a barcode of a" AND extractvalue(1,concat(0x7e,(SELECT @@version)))-- -.
  4. Confirm the request no longer returns a database error leaking server
    information, and that the injected value is treated as a literal barcode.
  5. Exercise normal biobank flows (create/edit containers, specimens, pools,
    shipments; cascade a temperature/status/center change to child containers)
    and confirm behaviour is unchanged.

Link to any issues this addresses

Reported via the Programme privé de prime aux bogues du Gouvernement du Québec
(YesWeHack). Internal refs: RITM0025010, RITM0024993.

@github-actions github-actions Bot added the Language: PHP PR or issue that update PHP code label Jun 22, 2026
@HenriRabalais HenriRabalais force-pushed the 2026-06-22_biobank-sql-injection-parameterize branch from 2b8fe0e to 50cafd1 Compare June 22, 2026 14:53
@HenriRabalais HenriRabalais added Category: Security PR or issue that aims to improve security Priority: High PR or issue should be prioritised over others for review and testing labels Jun 22, 2026
@ridz1208 ridz1208 changed the base branch from main to 28.0-release June 23, 2026 13:55
@ridz1208 ridz1208 changed the base branch from 28.0-release to main June 23, 2026 13:55
@skarya22

Copy link
Copy Markdown
Contributor

Following for once it is on 28.0-release

@HenriRabalais HenriRabalais force-pushed the 2026-06-22_biobank-sql-injection-parameterize branch from 7023824 to e959bad Compare June 23, 2026 18:21
@github-actions github-actions Bot added Module: dataquery PR or issue related to (new) dataquery module Multilingual Any tasks related to making LORIS multilingual labels Jun 23, 2026
@HenriRabalais HenriRabalais changed the base branch from main to 28.0-release June 23, 2026 18:22
@HenriRabalais

Copy link
Copy Markdown
Collaborator Author

@skarya22 done!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Category: Security PR or issue that aims to improve security Language: PHP PR or issue that update PHP code Module: dataquery PR or issue related to (new) dataquery module Multilingual Any tasks related to making LORIS multilingual Priority: High PR or issue should be prioritised over others for review and testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants