Skip to content

Commit 50cafd1

Browse files
HenriRabalaisHenriRabalais
committed
Fix SQL injection in biobank DAO selectInstances and cascade update
1 parent d48d363 commit 50cafd1

4 files changed

Lines changed: 38 additions & 33 deletions

File tree

‎modules/biobank/php/containerdao.class.inc‎

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -134,18 +134,19 @@ class ContainerDAO extends \LORIS\Data\ProvisionerInstance
134134
ON bcsr.ShipmentID=s.ShipmentID
135135
LEFT JOIN biobank_specimen bs
136136
ON bc.ContainerID=bs.ContainerID';
137+
$params = [];
137138
if (!empty($conditions)) {
138139
$whereClause = [];
140+
$i = 0;
139141
foreach ($conditions as $condition) {
140-
$whereClause[] = $condition['column']
141-
. '="'
142-
. $condition['value']
143-
. '"';
142+
$key = 'param'.$i++;
143+
$whereClause[] = $condition['column'].'=:'.$key;
144+
$params[$key] = $condition['value'];
144145
}
145146
$query .= ' WHERE '.implode(" $operator ", $whereClause);
146147
}
147148
$query .= ' GROUP BY bc.ContainerID';
148-
$containerRows = $this->db->pselectWithIndexKey($query, [], 'ContainerID');
149+
$containerRows = $this->db->pselectWithIndexKey($query, $params, 'ContainerID');
149150

150151
$containers = [];
151152
foreach ($containerRows as $id => $containerRow) {
@@ -213,7 +214,7 @@ class ContainerDAO extends \LORIS\Data\ProvisionerInstance
213214
{
214215
$query = 'SELECT ContainerCapacityID as id,
215216
Quantity as quantity,
216-
UnitID as unitId
217+
UnitID as unitId
217218
FROM biobank_container_capacity';
218219
$capacities = $this->db->pselectWithIndexKey($query, [], 'id');
219220

@@ -234,7 +235,7 @@ class ContainerDAO extends \LORIS\Data\ProvisionerInstance
234235
public function getUnits() : array
235236
{
236237
$query = "SELECT UnitID as id,
237-
Label as unit
238+
Label as unit
238239
FROM biobank_unit";
239240
$units = $this->db->pselectWithIndexKey($query, [], 'id');
240241

@@ -482,10 +483,14 @@ class ContainerDAO extends \LORIS\Data\ProvisionerInstance
482483
if (empty($childContainerIds)) {
483484
return [];
484485
}
485-
$query = "UPDATE biobank_container
486-
SET $field=$value
487-
WHERE ContainerID IN (".implode(',', $childContainerIds).");";
488-
$this->db->run($query);
486+
$query = "UPDATE biobank_container
487+
SET $field=:value
488+
WHERE ContainerID IN (".implode(',', $childContainerIds).")";
489+
$this->db->execute(
490+
$this->db->prepare($query),
491+
['value' => $value],
492+
['nofetch' => true]
493+
);
489494
$conditions = [];
490495
foreach ($childContainerIds as $id) {
491496
$conditions[] = ['column' => 'bc.ContainerID', 'value' => $id];
@@ -507,12 +512,12 @@ class ContainerDAO extends \LORIS\Data\ProvisionerInstance
507512
*/
508513
public function getAllChildContainerIds(Container $container) : array
509514
{
510-
$query = 'WITH recursive cte (ContainerID) as
515+
$query = 'WITH recursive cte (ContainerID) as
511516
(
512517
SELECT ContainerID
513518
FROM biobank_container_parent
514519
WHERE ParentContainerID=:i
515-
UNION ALL
520+
UNION ALL
516521
SELECT child.ContainerID
517522
FROM biobank_container_parent as child
518523
INNER JOIN cte on cte.ContainerID=child.ParentContainerID

‎modules/biobank/php/pooldao.class.inc‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -102,19 +102,19 @@ class PoolDAO extends \LORIS\Data\ProvisionerInstance
102102
biobank_specimen bs ON bs.SpecimenID=bspr.SpecimenID
103103
LEFT JOIN
104104
biobank_container bc ON bc.ContainerID=bs.ContainerID";
105+
$params = [];
105106
if (!empty($conditions)) {
106107
$whereClause = [];
108+
$i = 0;
107109
foreach ($conditions as $condition) {
108-
$whereClause[] = $condition['column']
109-
. '='
110-
. '"'
111-
. $condition['value']
112-
. '"';
110+
$key = 'param'.$i++;
111+
$whereClause[] = $condition['column'].'=:'.$key;
112+
$params[$key] = $condition['value'];
113113
}
114114
$query .= ' WHERE '.implode(" $operator ", $whereClause);
115115
}
116116
$query .= ' GROUP BY bp.PoolID';
117-
$poolRows = $this->db->pselectWithIndexKey($query, [], 'PoolID');
117+
$poolRows = $this->db->pselectWithIndexKey($query, $params, 'PoolID');
118118

119119
$pools = [];
120120
$specimenDAO = new SpecimenDAO($this->loris);

‎modules/biobank/php/shipmentdao.class.inc‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -109,21 +109,21 @@ class ShipmentDAO extends \LORIS\Data\ProvisionerInstance
109109
ON (s.ShipmentID=bcsr.ShipmentID)
110110
LEFT JOIN biobank_container bc
111111
ON (bcsr.ContainerID=bc.ContainerID)";
112+
$params = [];
112113
if (!empty($conditions)) {
113114
$whereClause = [];
115+
$i = 0;
114116
foreach ($conditions as $condition) {
115-
$whereClause[] = $condition['column']
116-
. '='
117-
. '"'
118-
. $condition['value']
119-
. '"';
117+
$key = 'param'.$i++;
118+
$whereClause[] = $condition['column'].'=:'.$key;
119+
$params[$key] = $condition['value'];
120120
}
121121
$query .= ' WHERE '.implode(" $operator ", $whereClause);
122122
}
123-
$query .= " GROUP BY s.ShipmentID";
123+
$query .= " GROUP BY s.ShipmentID";
124+
$shipmentRows = $this->db->pselectWithIndexKey($query, $params, 'barcode');
124125

125126
$shipments = [];
126-
$shipmentRows = $this->db->pselectWithIndexKey($query, [], 'barcode');
127127
$logs = $this->getLogs();
128128
foreach ($shipmentRows as $barcode => $shipmentRow) {
129129
$shipmentRow['containerIds'] = $shipmentRow['containerIds'] ?

‎modules/biobank/php/specimendao.class.inc‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -169,19 +169,19 @@ class SpecimenDAO extends \LORIS\Data\ProvisionerInstance
169169
ON bspa.ParentSpecimenID=bs2.SpecimenID
170170
LEFT JOIN biobank_container bc2
171171
ON bs2.ContainerID=bc2.ContainerID";
172+
$params = [];
172173
if (!empty($conditions)) {
173174
$whereClause = [];
175+
$i = 0;
174176
foreach ($conditions as $condition) {
175-
$whereClause[] = $condition['column']
176-
. '='
177-
. '"'
178-
. $condition['value']
179-
. '"';
177+
$key = 'param'.$i++;
178+
$whereClause[] = $condition['column'].'=:'.$key;
179+
$params[$key] = $condition['value'];
180180
}
181181
$query .= ' WHERE '.implode(" $operator ", $whereClause);
182182
}
183183
$query .= " GROUP BY bs.SpecimenID";
184-
$specimenRows = $this->db->pselectWithIndexKey($query, [], 'SpecimenID');
184+
$specimenRows = $this->db->pselectWithIndexKey($query, $params, 'SpecimenID');
185185
$specimens = [];
186186
foreach ($specimenRows as $id => $specimenRow) {
187187
$specimens[$id] = $this->_getInstanceFromSQL($specimenRow);
@@ -374,7 +374,7 @@ class SpecimenDAO extends \LORIS\Data\ProvisionerInstance
374374
public function getProtocolAttributes() : array
375375
{
376376
$query = "
377-
SELECT
377+
SELECT
378378
bsp.SpecimenProtocolID as protocolId,
379379
bsa.SpecimenAttributeID as attributeId,
380380
bsa.Label as label,

0 commit comments

Comments
 (0)