From 0ae896beba4392d5fb5eff0f35a928d6b1e60190 Mon Sep 17 00:00:00 2001 From: Carl Schwan Date: Wed, 31 Aug 2022 21:15:02 +0200 Subject: [PATCH] Add plsam taint analysis baseline And remove some false positive Signed-off-by: Carl Schwan --- .github/workflows/static-code-analysis.yml | 26 +++ .../lib/Controller/LayoutApiController.php | 1 + build/psalm-baseline-taint.xml | 153 ++++++++++++++++++ composer.json | 3 +- lib/private/Files/Cache/Cache.php | 3 +- lib/private/Files/Filesystem.php | 1 + lib/private/legacy/OC_App.php | 4 +- psalm-taint.xml | 132 +++++++++++++++ 8 files changed, 320 insertions(+), 3 deletions(-) create mode 100644 build/psalm-baseline-taint.xml create mode 100644 psalm-taint.xml diff --git a/.github/workflows/static-code-analysis.yml b/.github/workflows/static-code-analysis.yml index 31ee0f89fa203..55e0fb6784071 100644 --- a/.github/workflows/static-code-analysis.yml +++ b/.github/workflows/static-code-analysis.yml @@ -54,3 +54,29 @@ jobs: - name: Show potential changes in Psalm baseline run: | bash -c "[[ ! \"`git status --porcelain build/psalm-baseline-ocp.xml`\" ]] || ( echo 'Uncommited changes in Psalm baseline' && git status && git diff build/psalm-baseline.xml)" + + static-code-analysis-taint: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - name: Checkout submodules + shell: bash + run: | + auth_header="$(git config --local --get http.https://github.com/.extraheader)" + git submodule sync --recursive + git -c "http.extraheader=$auth_header" -c protocol.version=2 submodule update --init --force --recursive --depth=1 + - name: Set up php7.4 + uses: shivammathur/setup-php@master + with: + php-version: 7.4 + extensions: ctype,curl,dom,fileinfo,gd,intl,json,mbstring,openssl,pdo_sqlite,posix,sqlite,xml,zip + coverage: none + - name: Composer install + run: composer i + - name: Psalm + run: composer run psalm -- -c psalm-taint.xml --monochrome --no-progress --output-format=github --update-baseline || ( git diff -- . ':!lib/composer' && exit 1 ) + - name: Check diff + run: git diff -- . ':!lib/composer' + - name: Show potential changes in Psalm baseline + run: | + bash -c "[[ ! \"`git status --porcelain build/psalm-baseline-ocp.xml`\" ]] || ( echo 'Uncommited changes in Psalm baseline' && git status && git diff build/psalm-baseline.xml)" diff --git a/apps/dashboard/lib/Controller/LayoutApiController.php b/apps/dashboard/lib/Controller/LayoutApiController.php index 755470b7b07ea..b78eec6c853fb 100644 --- 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 new file mode 100644 index 0000000000000..d1e74e019262d --- /dev/null +++ b/build/psalm-baseline-taint.xml @@ -0,0 +1,153 @@ + + + + + $params + + + + + $keyPath + $keyPath + + + + + $downloadStartSecret + + + + + $appIcon + $imageFile + + + + + 'Location: '.\OC::$WEBROOT.'/' + 'Location: '.\OC::$WEBROOT.'/' + + + + + $this->cache + + + + + $this->systemConfig->getValue('query_log_file', '') + + + + + $this->logFile + $this->logFile + + + + + $file + $file + $file + $file + $this->folder + $this->getIndexFilename() + + + + + $this->passphrase + + + + + $dataDir + $dataDir + + + + + $sqliteFile + + + + + $body + $body + + + $body + $body + + + + + $themePath + + + + + $response + $response + 'data: ' . OC_JSON::encode($data) . PHP_EOL + 'data: ' . OC_JSON::encode($data) . PHP_EOL + + + $response + $response + 'data: ' . OC_JSON::encode($data) . PHP_EOL + 'data: ' . OC_JSON::encode($data) . PHP_EOL + + + + + sprintf('Content-Range: bytes %d-%d/%d', $rangeArray[0]['from'], $rangeArray[0]['to'], $fileSize) + + + + + + + self::encode($data) + self::encode($data) + self::encode($data) + self::encode($data) + + + self::encode($data) + self::encode($data) + self::encode($data) + self::encode($data) + + + + + $CONFIG_DATADIRECTORY + $testFile + $testFile + + + + + $sql + $sql + $sql + $sql + + + + + $rotatedFile + $this->filePath + + + + + $controller->buildProviderList()->render() + + + $controller->buildProviderList()->render() + $controller->buildProviderList()->render() + + + diff --git a/composer.json b/composer.json index 6a8e9e9956967..5a4eb36fdf551 100644 --- 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 index ec284282178e9..5736e975b97ad 100644 --- 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 index b9b9534b15f45..fd2ed9366f3f1 100644 --- a/lib/private/Files/Filesystem.php +++ b/lib/private/Files/Filesystem.php @@ -672,6 +672,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 index a5eb26d0d4ba2..b19653d5916fd 100644 --- 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 new file mode 100644 index 0000000000000..ca22010504146 --- /dev/null +++ b/psalm-taint.xml @@ -0,0 +1,132 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +