Add Phan plugin to check for SQL injections

This adds a phan plugin which checks for SQL injections on code using our QueryBuilder, while it isn't perfect it should already catch most potential issues.

As always, static analysis will sometimes have false positives and this is also here the case. So in some cases the analyzer just doesn't know if something is potential user input or not, thus I had to add some `@suppress SqlInjectionChecker` in front of those potential injections.

The Phan plugin hasn't the most awesome code but it works and I also added a file with test cases.

Signed-off-by: Lukas Reschke <lukas@statuscode.ch>

14 files changed, 31 insertions, 0 deletions

diff --git a/lib/private/BackgroundJob/JobList.php b/lib/private/BackgroundJob/JobList.php
index b0c580290ed..366fe8aac03 100644
--- a/lib/private/BackgroundJob/JobList.php
+++ b/lib/private/BackgroundJob/JobList.php
@@ -284,6 +284,7 @@ class JobList implements IJobList {
 	 * Remove the reservation for a job
 	 *
 	 * @param IJob $job
+	 * @suppress SqlInjectionChecker
 	 */
 	public function unlockJob(IJob $job) {
 		$query = $this->connection->getQueryBuilder();
diff --git a/lib/private/Comments/Manager.php b/lib/private/Comments/Manager.php
index f1c5b7dca50..4645adc6c5b 100644
--- a/lib/private/Comments/Manager.php
+++ b/lib/private/Comments/Manager.php
@@ -691,6 +691,7 @@ class Manager implements ICommentsManager {
 	 * @param \DateTime $dateTime
 	 * @param IUser $user
 	 * @since 9.0.0
+	 * @suppress SqlInjectionChecker
 	 */
 	public function setReadMark($objectType, $objectId, \DateTime $dateTime, IUser $user) {
 		$this->checkRoleParameters('Object', $objectType, $objectId);
diff --git a/lib/private/DB/Connection.php b/lib/private/DB/Connection.php
index 563c077b04a..4f042e403f1 100644
--- a/lib/private/DB/Connection.php
+++ b/lib/private/DB/Connection.php
@@ -272,6 +272,7 @@ class Connection extends \Doctrine\DBAL\Connection implements IDBConnection {
 	 * @return int number of new rows
 	 * @throws \Doctrine\DBAL\DBALException
 	 * @throws PreConditionNotMetException
+	 * @suppress SqlInjectionChecker
 	 */
 	public function setValues($table, array $keys, array $values, array $updatePreconditionValues = []) {
 		try {
diff --git a/lib/private/Files/Cache/Cache.php b/lib/private/Files/Cache/Cache.php
index 1f3f2433e45..b2f379fe2c2 100644
--- a/lib/private/Files/Cache/Cache.php
+++ b/lib/private/Files/Cache/Cache.php
@@ -502,6 +502,7 @@ class Cache implements ICache {
 	 * @param string $targetPath
 	 * @throws \OC\DatabaseException
 	 * @throws \Exception if the given storages have an invalid id
+	 * @suppress SqlInjectionChecker
 	 */
 	public function moveFromCache(ICache $sourceCache, $sourcePath, $targetPath) {
 		if ($sourceCache instanceof Cache) {
diff --git a/lib/private/Files/Cache/Propagator.php b/lib/private/Files/Cache/Propagator.php
index d597a479f54..be69d174a0d 100644
--- a/lib/private/Files/Cache/Propagator.php
+++ b/lib/private/Files/Cache/Propagator.php
@@ -58,6 +58,7 @@ class Propagator implements IPropagator {
 	 * @param string $internalPath
 	 * @param int $time
 	 * @param int $sizeDifference number of bytes the file has grown
+	 * @suppress SqlInjectionChecker
 	 */
 	public function propagateChange($internalPath, $time, $sizeDifference = 0) {
 		$storageId = (int)$this->storage->getStorageCache()->getNumericId();
@@ -140,6 +141,7 @@ class Propagator implements IPropagator {
 
 	/**
 	 * Commit the active propagation batch
+	 * @suppress SqlInjectionChecker
 	 */
 	public function commitBatch() {
 		if (!$this->inBatch) {
diff --git a/lib/private/Files/Config/UserMountCache.php b/lib/private/Files/Config/UserMountCache.php
index 80cedfa1ccd..7ecad6e8cfb 100644
--- a/lib/private/Files/Config/UserMountCache.php
+++ b/lib/private/Files/Config/UserMountCache.php
@@ -334,6 +334,11 @@ class UserMountCache implements IUserMountCache {
 		$query->execute();
 	}
 
+	/**
+	 * @param array $users
+	 * @return array
+	 * @suppress SqlInjectionChecker
+	 */
 	public function getUsedSpaceForUsers(array $users) {
 		$builder = $this->connection->getQueryBuilder();
 
diff --git a/lib/private/Lock/DBLockingProvider.php b/lib/private/Lock/DBLockingProvider.php
index c521bcf548b..f4778a35fa8 100644
--- a/lib/private/Lock/DBLockingProvider.php
+++ b/lib/private/Lock/DBLockingProvider.php
@@ -255,6 +255,7 @@ class DBLockingProvider extends AbstractLockingProvider {
 
 	/**
 	 * release all lock acquired by this instance which were marked using the mark* methods
+	 * @suppress SqlInjectionChecker
 	 */
 	public function releaseAll() {
 		parent::releaseAll();
diff --git a/lib/private/Repair/CleanTags.php b/lib/private/Repair/CleanTags.php
index 9b44fb1e671..d3bea0f9957 100644
--- a/lib/private/Repair/CleanTags.php
+++ b/lib/private/Repair/CleanTags.php
@@ -167,6 +167,7 @@ class CleanTags implements IRepairStep {
 	 * @param string $sourceId
 	 * @param string $sourceNullColumn	If this column is null in the source table,
 	 * 								the entry is deleted in the $deleteTable
+	 * @suppress SqlInjectionChecker
 	 */
 	protected function deleteOrphanEntries(IOutput $output, $repairInfo, $deleteTable, $deleteId, $sourceTable, $sourceId, $sourceNullColumn) {
 		$qb = $this->connection->getQueryBuilder();
diff --git a/lib/private/Repair/NC13/RepairInvalidPaths.php b/lib/private/Repair/NC13/RepairInvalidPaths.php
index efc682bf44f..cbbbc82801c 100644
--- a/lib/private/Repair/NC13/RepairInvalidPaths.php
+++ b/lib/private/Repair/NC13/RepairInvalidPaths.php
@@ -51,6 +51,10 @@ class RepairInvalidPaths implements IRepairStep {
 		return 'Repair invalid paths in file cache';
 	}
 
+	/**
+	 * @return \Generator
+	 * @suppress SqlInjectionChecker
+	 */
 	private function getInvalidEntries() {
 		$builder = $this->connection->getQueryBuilder();
 
@@ -95,6 +99,11 @@ class RepairInvalidPaths implements IRepairStep {
 		return $this->getIdQuery->execute()->fetchColumn();
 	}
 
+	/**
+	 * @param string $fileid
+	 * @param string $newPath
+	 * @suppress SqlInjectionChecker
+	 */
 	private function update($fileid, $newPath) {
 		if (!$this->updateQuery) {
 			$builder = $this->connection->getQueryBuilder();
diff --git a/lib/private/Repair/OldGroupMembershipShares.php b/lib/private/Repair/OldGroupMembershipShares.php
index ea0256f64b8..5b941d1fcbd 100644
--- a/lib/private/Repair/OldGroupMembershipShares.php
+++ b/lib/private/Repair/OldGroupMembershipShares.php
@@ -65,6 +65,7 @@ class OldGroupMembershipShares implements IRepairStep {
 	 * Must throw exception on error.
 	 *
 	 * @throws \Exception in case of failure
+	 * @suppress SqlInjectionChecker
 	 */
 	public function run(IOutput $output) {
 		$deletedEntries = 0;
diff --git a/lib/private/Repair/RepairInvalidShares.php b/lib/private/Repair/RepairInvalidShares.php
index 78884ca9cde..92423165541 100644
--- a/lib/private/Repair/RepairInvalidShares.php
+++ b/lib/private/Repair/RepairInvalidShares.php
@@ -56,6 +56,7 @@ class RepairInvalidShares implements IRepairStep {
 
 	/**
 	 * Adjust file share permissions
+	 * @suppress SqlInjectionChecker
 	 */
 	private function adjustFileSharePermissions(IOutput $out) {
 		$mask = \OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_UPDATE | \OCP\Constants::PERMISSION_SHARE;
diff --git a/lib/private/Security/Bruteforce/Throttler.php b/lib/private/Security/Bruteforce/Throttler.php
index ee02bc5a1c4..f495baf1924 100644
--- a/lib/private/Security/Bruteforce/Throttler.php
+++ b/lib/private/Security/Bruteforce/Throttler.php
@@ -89,6 +89,7 @@ class Throttler {
 	 * @param string $action
 	 * @param string $ip
 	 * @param array $metadata Optional metadata logged to the database
+	 * @suppress SqlInjectionChecker
 	 */
 	public function registerAttempt($action,
 									$ip,
diff --git a/lib/private/Settings/Mapper.php b/lib/private/Settings/Mapper.php
index 3219a812cd5..ceb68c9eebf 100644
--- a/lib/private/Settings/Mapper.php
+++ b/lib/private/Settings/Mapper.php
@@ -198,6 +198,7 @@ class Mapper {
 	 * @param string $idCol
 	 * @param string $id
 	 * @param array $values
+	 * @suppress SqlInjectionChecker
 	 */
 	public function update($table, $idCol, $id, $values) {
 		$query = $this->dbc->getQueryBuilder();
diff --git a/lib/private/Setup/PostgreSQL.php b/lib/private/Setup/PostgreSQL.php
index 18ed9fcdef6..8267b065142 100644
--- a/lib/private/Setup/PostgreSQL.php
+++ b/lib/private/Setup/PostgreSQL.php
@@ -34,6 +34,11 @@ use OCP\IDBConnection;
 class PostgreSQL extends AbstractDatabase {
 	public $dbprettyname = 'PostgreSQL';
 
+	/**
+	 * @param string $username
+	 * @throws \OC\DatabaseSetupException
+	 * @suppress SqlInjectionChecker
+	 */
 	public function setupDatabase($username) {
 		try {
 			$connection = $this->connect([