Brian Wolff has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/395753 )
Change subject: Fix handling of IN(...) lists in db select wrapper
......................................................................
Fix handling of IN(...) lists in db select wrapper
Change-Id: I5685e37e0ac032f8a364ff67a06f0645490c2f40
---
M src/TaintednessVisitor.php
A tests/integration/arraynumkey2/db.php
A tests/integration/arraynumkey2/expectedResults.txt
A tests/integration/arraynumkey2/test.php
4 files changed, 67 insertions(+), 1 deletion(-)
diff --git a/src/TaintednessVisitor.php b/src/TaintednessVisitor.php
index 2d45a58..3451e45 100644
--- a/src/TaintednessVisitor.php
+++ b/src/TaintednessVisitor.php
@@ -328,7 +328,11 @@
$var = $node->children['var'];
if ( $var->kind === \ast\AST_DIM ) {
$dim = $var->children['dim'];
- if ( $rhsTaintedness & SecurityCheckPlugin::SQL_TAINT )
{
+ if ( $rhsTaintedness &
SecurityCheckPlugin::SQL_NUMKEY_TAINT ) {
+ // Things like 'foo' => ['taint', 'taint']
+ // are ok.
+ $rhsTaintedness &=
~SecurityCheckPlugin::SQL_NUMKEY_TAINT;
+ } elseif ( $rhsTaintedness &
SecurityCheckPlugin::SQL_TAINT ) {
if (
$dim === null ||
$this->nodeIsInt( $dim )
@@ -764,6 +768,12 @@
$value = $child->children['value'];
$sqlTaint = SecurityCheckPlugin::SQL_TAINT;
if (
+ $this->getTaintedness( $value )
+ & SecurityCheckPlugin::SQL_NUMKEY_TAINT
+ ) {
+ $curTaint &=
~SecurityCheckPlugin::SQL_NUMKEY_TAINT;
+
+ } elseif (
( $this->getTaintedness( $key ) & $sqlTaint ) ||
( ( $key === null || $this->nodeIsInt( $key ) )
&& ( $this->getTaintedness( $value ) &
$sqlTaint ) )
diff --git a/tests/integration/arraynumkey2/db.php
b/tests/integration/arraynumkey2/db.php
new file mode 100644
index 0000000..c607198
--- /dev/null
+++ b/tests/integration/arraynumkey2/db.php
@@ -0,0 +1,26 @@
+<?php
+namespace Wikimedia\Rdbms;
+
+interface IDatabase {
+ public function query( $sql, $method );
+}
+
+class Database implements IDatabase {
+ public function query( $sql, $method ) {
+ // do some stuff
+ return (object)[ 'some_field' => 'some value' ];
+ }
+
+ public function select(
+ $table, $vars, $conds = '', $fname = __METHOD__,
+ $options = [], $join_conds = []
+ ) {
+ return (object)[ 'some_field' => 'some value' ];
+ }
+}
+
+class MysqlDatabase extends Database {
+ public function getType() {
+ return 'mysql';
+ }
+}
diff --git a/tests/integration/arraynumkey2/expectedResults.txt
b/tests/integration/arraynumkey2/expectedResults.txt
new file mode 100644
index 0000000..60d4fd2
--- /dev/null
+++ b/tests/integration/arraynumkey2/expectedResults.txt
@@ -0,0 +1 @@
+integration/arraynumkey2/test.php:29 SecurityCheck-SQLInjection Calling method
\Wikimedia\Rdbms\MysqlDatabase::select() in [no method] that outputs using
tainted argument $list. (Caused by: integration/arraynumkey2/test.php +22;
integration/arraynumkey2/test.php +23; integration/arraynumkey2/test.php +24)
diff --git a/tests/integration/arraynumkey2/test.php
b/tests/integration/arraynumkey2/test.php
new file mode 100644
index 0000000..2b0092d
--- /dev/null
+++ b/tests/integration/arraynumkey2/test.php
@@ -0,0 +1,29 @@
+<?php
+
+use Wikimedia\Rdbms\MysqlDatabase;
+
+$db = new MysqlDatabase;
+
+// safe
+$db->select(
+ 'table',
+ 'field',
+ [ 'foo' => $_GET['bar'] ]
+);
+
+// safe
+$db->select(
+ 'table',
+ 'field',
+ [ 'foo' => [ $_GET['bar'], $_GET['baz'] ] ]
+);
+
+$list = [];
+$list[] = $_GET['bar'];
+$list[] = $_GET['baz'];
+$list[] = $_GET['fred'];
+
+// safe
+$db->select( [ 'table', 'table2' ], [ 'field1', 'field2' ], [ 'foo' => $list
], __METHOD__ );
+// unsafe
+$db->select( [ 'table', 'table2' ], [ 'field1', 'field2' ], $list, __METHOD__
);
--
To view, visit https://gerrit.wikimedia.org/r/395753
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I5685e37e0ac032f8a364ff67a06f0645490c2f40
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/tools/phan/SecurityCheckPlugin
Gerrit-Branch: master
Gerrit-Owner: Brian Wolff <bawolff...@gmail.com>
Gerrit-Reviewer: Brian Wolff <bawolff...@gmail.com>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
