Add plsam taint analysis baseline

And remove some false positive Signed-off-by: Carl Schwan <carl@carlschwan.eu>
nextcloud · Aug 31, 2022 · a6734be · a6734be
1 parent b617f39
commit a6734be
Show file tree

Hide file tree

Showing 7 changed files with 294 additions and 3 deletions.
diff --git a/apps/dashboard/lib/Controller/LayoutApiController.php b/apps/dashboard/lib/Controller/LayoutApiController.php
@@ -56,6 +56,7 @@ public function __construct(
 	 * @return JSONResponse
 	 */
 	public function create(string $layout): JSONResponse {
+		$layout = htmlspecialchars($layout);
 		$this->config->setUserValue($this->userId, 'dashboard', 'layout', $layout);
 		return new JSONResponse(['layout' => $layout]);
 	}

diff --git a/build/psalm-baseline-taint.xml b/build/psalm-baseline-taint.xml
@@ -0,0 +1,153 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<files psalm-version="4.18.1@dda05fa913f4dc6eb3386f2f7ce5a45d37a71bcb">
+  <file src="apps/admin_audit/lib/Actions/Action.php">
+    <TaintedHtml occurrences="1">
+      <code>$params</code>
+    </TaintedHtml>
+  </file>
+  <file src="apps/files_external/lib/Lib/Storage/SFTP.php">
+    <TaintedFile occurrences="2">
+      <code>$keyPath</code>
+      <code>$keyPath</code>
+    </TaintedFile>
+  </file>
+  <file src="apps/files_sharing/lib/Controller/ShareController.php">
+    <TaintedCookie occurrences="1">
+      <code>$downloadStartSecret</code>
+    </TaintedCookie>
+  </file>
+  <file src="apps/theming/lib/IconBuilder.php">
+    <TaintedFile occurrences="2">
+      <code>$appIcon</code>
+      <code>$imageFile</code>
+    </TaintedFile>
+  </file>
+  <file src="lib/base.php">
+    <TaintedHeader occurrences="2">
+      <code>'Location: '.\OC::$WEBROOT.'/'</code>
+      <code>'Location: '.\OC::$WEBROOT.'/'</code>
+    </TaintedHeader>
+  </file>
+  <file src="lib/private/Config.php">
+    <TaintedHtml occurrences="1">
+      <code>$this-&gt;cache</code>
+    </TaintedHtml>
+  </file>
+  <file src="lib/private/DB/Connection.php">
+    <TaintedFile occurrences="1">
+      <code>$this-&gt;systemConfig-&gt;getValue('query_log_file', '')</code>
+    </TaintedFile>
+  </file>
+  <file src="lib/private/Log/File.php">
+    <TaintedFile occurrences="2">
+      <code>$this-&gt;logFile</code>
+      <code>$this-&gt;logFile</code>
+    </TaintedFile>
+  </file>
+  <file src="lib/private/Profiler/FileProfilerStorage.php">
+    <TaintedFile occurrences="6">
+      <code>$file</code>
+      <code>$file</code>
+      <code>$file</code>
+      <code>$file</code>
+      <code>$this-&gt;folder</code>
+      <code>$this-&gt;getIndexFilename()</code>
+    </TaintedFile>
+  </file>
+  <file src="lib/private/Session/CryptoWrapper.php">
+    <TaintedCookie occurrences="1">
+      <code>$this-&gt;passphrase</code>
+    </TaintedCookie>
+  </file>
+  <file src="lib/private/Setup.php">
+    <TaintedFile occurrences="2">
+      <code>$dataDir</code>
+      <code>$dataDir</code>
+    </TaintedFile>
+  </file>
+  <file src="lib/private/Setup/Sqlite.php">
+    <TaintedFile occurrences="1">
+      <code>$sqliteFile</code>
+    </TaintedFile>
+  </file>
+  <file src="lib/private/legacy/OC_API.php">
+    <TaintedHtml occurrences="2">
+      <code>$body</code>
+      <code>$body</code>
+    </TaintedHtml>
+    <TaintedTextWithQuotes occurrences="2">
+      <code>$body</code>
+      <code>$body</code>
+    </TaintedTextWithQuotes>
+  </file>
+  <file src="lib/private/legacy/OC_Defaults.php">
+    <TaintedInclude occurrences="1">
+      <code>$themePath</code>
+    </TaintedInclude>
+  </file>
+  <file src="lib/private/legacy/OC_EventSource.php">
+    <TaintedHtml occurrences="4">
+      <code>$response</code>
+      <code>$response</code>
+      <code>'data: ' . OC_JSON::encode($data) . PHP_EOL</code>
+      <code>'data: ' . OC_JSON::encode($data) . PHP_EOL</code>
+    </TaintedHtml>
+    <TaintedTextWithQuotes occurrences="4">
+      <code>$response</code>
+      <code>$response</code>
+      <code>'data: ' . OC_JSON::encode($data) . PHP_EOL</code>
+      <code>'data: ' . OC_JSON::encode($data) . PHP_EOL</code>
+    </TaintedTextWithQuotes>
+  </file>
+  <file src="lib/private/legacy/OC_Files.php">
+    <TaintedHeader occurrences="1">
+      <code>sprintf('Content-Range: bytes %d-%d/%d', $rangeArray[0]['from'], $rangeArray[0]['to'], $fileSize)</code>
+    </TaintedHeader>
+    <TaintedHtml occurrences="1"/>
+    <TaintedTextWithQuotes occurrences="1"/>
+  </file>
+  <file src="lib/private/legacy/OC_JSON.php">
+    <TaintedHtml occurrences="4">
+      <code>self::encode($data)</code>
+      <code>self::encode($data)</code>
+      <code>self::encode($data)</code>
+      <code>self::encode($data)</code>
+    </TaintedHtml>
+    <TaintedTextWithQuotes occurrences="4">
+      <code>self::encode($data)</code>
+      <code>self::encode($data)</code>
+      <code>self::encode($data)</code>
+      <code>self::encode($data)</code>
+    </TaintedTextWithQuotes>
+  </file>
+  <file src="lib/private/legacy/OC_Util.php">
+    <TaintedFile occurrences="3">
+      <code>$CONFIG_DATADIRECTORY</code>
+      <code>$testFile</code>
+      <code>$testFile</code>
+    </TaintedFile>
+  </file>
+  <file src="lib/public/IDBConnection.php">
+    <TaintedSql occurrences="4">
+      <code>$sql</code>
+      <code>$sql</code>
+      <code>$sql</code>
+      <code>$sql</code>
+    </TaintedSql>
+  </file>
+  <file src="lib/public/Log/RotationTrait.php">
+    <TaintedFile occurrences="2">
+      <code>$rotatedFile</code>
+      <code>$this-&gt;filePath</code>
+    </TaintedFile>
+  </file>
+  <file src="ocs-provider/index.php">
+    <TaintedHtml occurrences="1">
+      <code>$controller-&gt;buildProviderList()-&gt;render()</code>
+    </TaintedHtml>
+    <TaintedTextWithQuotes occurrences="2">
+      <code>$controller-&gt;buildProviderList()-&gt;render()</code>
+      <code>$controller-&gt;buildProviderList()-&gt;render()</code>
+    </TaintedTextWithQuotes>
+  </file>
+</files>
diff --git a/composer.json b/composer.json
@@ -46,6 +46,7 @@
 		"cs:check": "php-cs-fixer fix --dry-run --diff",
 		"lint": "find . -name \\*.php -not -path './lib/composer/*' -not -path './build/stubs/*' -print0 | xargs -0 -n1 php -l",
 		"psalm": "psalm --threads=1",
-		"psalm:update-baseline": "psalm --threads=1 --update-baseline"
+		"psalm:update-baseline": "psalm --threads=1 --update-baseline",
+		"psalm:taint": "psalm -c psalm-taint.xml --threads=1 --use-baseline=build/psalm-baseline-taint.xml --taint-analysis"
 	}
 }
diff --git a/lib/private/Files/Cache/Cache.php b/lib/private/Files/Cache/Cache.php
@@ -422,8 +422,9 @@ public function update($id, array $data) {
 	}
 
 	/**
-	 * extract query parts and params array from data array
+	 * Extract query parts and params array from data array
 	 *
+	 * @psalm-taint-escape sql
 	 * @param array $data
 	 * @return array
 	 */

diff --git a/lib/private/Files/Filesystem.php b/lib/private/Files/Filesystem.php
@@ -691,6 +691,7 @@ public static function hasUpdated($path, $time) {
 	 * @param bool $stripTrailingSlash whether to strip the trailing slash
 	 * @param bool $isAbsolutePath whether the given path is absolute
 	 * @param bool $keepUnicode true to disable unicode normalization
+	 * @psalm-taint-escape file
 	 * @return string
 	 */
 	public static function normalizePath($path, $stripTrailingSlash = true, $isAbsolutePath = false, $keepUnicode = false) {

diff --git a/lib/private/legacy/OC_App.php b/lib/private/legacy/OC_App.php
@@ -85,12 +85,14 @@ class OC_App {
 	 *
 	 * @psalm-taint-escape file
 	 * @psalm-taint-escape include
+	 * @psalm-taint-escape html
+	 * @psalm-taint-escape has_quotes
 	 *
 	 * @param string $app AppId that needs to be cleaned
 	 * @return string
 	 */
 	public static function cleanAppId(string $app): string {
-		return str_replace(['\0', '/', '\\', '..'], '', $app);
+		return str_replace(['<', '>', '"', "'", '\0', '/', '\\', '..'], '', $app);
 	}
 
 	/**

diff --git a/psalm-taint.xml b/psalm-taint.xml
@@ -0,0 +1,132 @@
+<?xml version="1.0"?>
+<psalm
+	errorLevel="4"
+	resolveFromConfigFile="true"
+	xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+	xmlns="https://getpsalm.org/schema/config"
+	xsi:schemaLocation="https://getpsalm.org/schema/config"
+    errorBaseline="build/psalm-baseline-taint.xml"
+>
+	<plugins>
+		<plugin filename="build/psalm/AppFrameworkTainter.php" />
+	</plugins>
+	<projectFiles>
+		<directory name="apps/admin_audit"/>
+		<directory name="apps/cloud_federation_api"/>
+		<directory name="apps/comments"/>
+		<directory name="apps/contactsinteraction"/>
+		<directory name="apps/dashboard"/>
+		<directory name="apps/dav"/>
+		<directory name="apps/encryption"/>
+		<directory name="apps/federatedfilesharing"/>
+		<directory name="apps/federation"/>
+		<directory name="apps/files"/>
+		<directory name="apps/files_external"/>
+		<directory name="apps/files_sharing"/>
+		<directory name="apps/files_trashbin"/>
+		<directory name="apps/files_versions"/>
+		<directory name="apps/lookup_server_connector"/>
+		<directory name="apps/oauth2"/>
+		<directory name="apps/provisioning_api"/>
+		<directory name="apps/settings"/>
+		<directory name="apps/sharebymail"/>
+		<directory name="apps/systemtags"/>
+		<directory name="apps/testing"/>
+		<directory name="apps/theming"/>
+		<directory name="apps/twofactor_backupcodes"/>
+		<directory name="apps/updatenotification"/>
+		<directory name="apps/user_ldap"/>
+		<directory name="apps/user_status"/>
+		<directory name="apps/weather_status"/>
+		<directory name="apps/workflowengine"/>
+		<directory name="core"/>
+		<directory name="lib"/>
+		<directory name="ocm-provider"/>
+		<directory name="ocs"/>
+		<directory name="ocs-provider"/>
+		<file name="index.php"/>
+		<file name="public.php"/>
+		<file name="remote.php"/>
+		<file name="status.php"/>
+		<file name="version.php"/>
+		<ignoreFiles>
+			<directory name="apps/**/composer"/>
+			<directory name="apps/**/tests"/>
+			<directory name="apps/files_external/3rdparty"/>
+			<directory name="lib/composer"/>
+			<directory name="lib/l10n"/>
+		</ignoreFiles>
+	</projectFiles>
+	<extraFiles>
+		<directory name="3rdparty"/>
+		<directory name="apps/files_external/3rdparty"/>
+	</extraFiles>
+	<stubs>
+		<file name="build/stubs/apcu.php"/>
+		<file name="build/stubs/gd.php"/>
+		<file name="build/stubs/imagick.php"/>
+		<file name="build/stubs/intl.php"/>
+		<file name="build/stubs/IntlChar.php"/>
+		<file name="build/stubs/ldap.php"/>
+		<file name="build/stubs/memcached.php"/>
+		<file name="build/stubs/redis.php"/>
+		<file name="build/stubs/redis_cluster.php"/>
+		<file name="build/stubs/sftp.php"/>
+		<file name="build/stubs/ssh2.php"/>
+		<file name="build/stubs/xsl.php"/>
+		<file name="build/stubs/ftp.php"/>
+		<file name="build/stubs/pcntl.php"/>
+		<file name="build/stubs/zip.php"/>
+	</stubs>
+	<issueHandlers>
+		<UndefinedClass>
+			<errorLevel type="suppress">
+				<referencedClass name="OCA\GroupFolders\Mount\GroupFolderStorage"/>
+				<referencedClass name="OCA\TwoFactorNextcloudNotification\Controller\APIController"/>
+			</errorLevel>
+		</UndefinedClass>
+		<UndefinedFunction>
+			<errorLevel type="suppress">
+				<!-- template functions: https://github.com/nextcloud/server/blob/6e8e34fef920a073118c22111f0f31eb3b3a91dc/lib/private/legacy/template/functions.php -->
+				<referencedFunction name="p"/>
+				<referencedFunction name="emit_css_tag"/>
+				<referencedFunction name="emit_css_loading_tags"/>
+				<referencedFunction name="emit_script_tag"/>
+				<referencedFunction name="emit_script_loading_tags"/>
+				<referencedFunction name="print_unescaped"/>
+				<referencedFunction name="script"/>
+				<referencedFunction name="vendor_script"/>
+				<referencedFunction name="style"/>
+				<referencedFunction name="vendor_style"/>
+				<referencedFunction name="translation"/>
+				<referencedFunction name="component"/>
+				<referencedFunction name="link_to"/>
+				<referencedFunction name="link_to_docs"/>
+				<referencedFunction name="image_path"/>
+				<referencedFunction name="mimetype_icon"/>
+				<referencedFunction name="preview_icon"/>
+				<referencedFunction name="publicPreview_icon"/>
+				<referencedFunction name="human_file_size"/>
+				<referencedFunction name="strip_time"/>
+				<referencedFunction name="relative_modified_date"/>
+				<referencedFunction name="html_select_options"/>
+			</errorLevel>
+		</UndefinedFunction>
+		<UndefinedGlobalVariable>
+			<errorLevel type="suppress">
+				<referencedVariable name="$_"/>
+				<referencedVariable name="$l"/>
+				<referencedVariable name="$theme"/>
+				<!-- false positive: https://github.com/nextcloud/server/blob/cb057829f72c70e819f456edfadbb29d72dba832/lib/private/Console/Application.php#L92 -->
+				<file name="core/register_command.php" />
+			</errorLevel>
+		</UndefinedGlobalVariable>
+		<UndefinedDocblockClass>
+			<errorLevel type="suppress">
+				<!-- Helper classes for sharing API integration from other apps -->
+				<referencedClass name="OCA\Deck\Sharing\ShareAPIHelper" />
+				<referencedClass name="OCA\Talk\Share\Helper\DeletedShareAPIController" />
+			</errorLevel>
+		</UndefinedDocblockClass>
+	</issueHandlers>
+</psalm>