feat(InsecureUnserialize): Add a sniff for insecure use of unserialize(). (#3255726 by tbradbury)

coder_sniffer/DrupalPractice/Sniffs/FunctionCalls/InsecureUnserializeSniff.php

+<?php
+/**
+ * \DrupalPractice\Sniffs\FunctionCalls\InsecureUnserializeSniff
+ *
+ * @category PHP
+ * @package  PHP_CodeSniffer
+ * @link     http://pear.php.net/package/PHP_CodeSniffer
+ */
+
+namespace DrupalPractice\Sniffs\FunctionCalls;
+
+use PHP_CodeSniffer\Files\File;
+use Drupal\Sniffs\Semantics\FunctionCall;
+
+/**
+ * Check that unserialize() limits classes that may be unserialized.
+ *
+ * @category PHP
+ * @package  PHP_CodeSniffer
+ * @link     http://pear.php.net/package/PHP_CodeSniffer
+ */
+class InsecureUnserializeSniff extends FunctionCall
+{
+
+
+    /**
+     * Returns an array of function names this test wants to listen for.
+     *
+     * @return array<string>
+     */
+    public function registerFunctionNames()
+    {
+        return ['unserialize'];
+
+    }//end registerFunctionNames()
+
+
+    /**
+     * Processes this function call.
+     *
+     * @param \PHP_CodeSniffer\Files\File $phpcsFile    The file being scanned.
+     * @param int                         $stackPtr     The position of the function call in
+     *                                                  the stack.
+     * @param int                         $openBracket  The position of the opening
+     *                                                  parenthesis in the stack.
+     * @param int                         $closeBracket The position of the closing
+     *                                                  parenthesis in the stack.
+     *
+     * @return void
+     */
+    public function processFunctionCall(
+        File $phpcsFile,
+        $stackPtr,
+        $openBracket,
+        $closeBracket
+    ) {
+        $tokens   = $phpcsFile->getTokens();
+        $argument = $this->getArgument(2);
+        if ($argument === false) {
+            $this->fail($phpcsFile, $closeBracket);
+            return;
+        }
+
+        $allowedClassesKeyStart = $phpcsFile->findNext(T_CONSTANT_ENCAPSED_STRING, $argument['start'], $argument['end'], false, '\'allowed_classes\'');
+        if ($allowedClassesKeyStart === false) {
+            $allowedClassesKeyStart = $phpcsFile->findNext(T_CONSTANT_ENCAPSED_STRING, $argument['start'], $argument['end'], false, '"allowed_classes"');
+        }
+
+        if ($allowedClassesKeyStart === false) {
+            $this->fail($phpcsFile, $argument['end']);
+            return;
+        }
+
+        $allowedClassesArrow = $phpcsFile->findNext(T_DOUBLE_ARROW, $allowedClassesKeyStart, $argument['end'], false);
+        if ($allowedClassesArrow === false) {
+            $this->fail($phpcsFile, $argument['end']);
+            return;
+        }
+
+        $allowedClassesValue = $phpcsFile->findNext(T_WHITESPACE, ($allowedClassesArrow + 1), $argument['end'], true);
+        if ($tokens[$allowedClassesValue]['code'] === T_TRUE) {
+            $this->fail($phpcsFile, $allowedClassesValue);
+        }
+
+    }//end processFunctionCall()
+
+
+    /**
+     * Record a violation of the standard.
+     *
+     * @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned.
+     * @param int                         $position  The stack position of the violation.
+     *
+     * @return void
+     */
+    protected function fail(File $phpcsFile, int $position)
+    {
+        $phpcsFile->addError('unserialize() is insecure unless allowed classes are limited. Use a safe format like JSON or use the allowed_classes option.', $position, 'InsecureUnserialize');
+
+    }//end fail()
+
+
+}//end class

tests/DrupalPractice/FunctionCalls/InsecureUnserializeUnitTest.inc

+<?php
+
+// Secure usages should have no errors.
+unserialize($foo, ['allowed_classes' => FALSE]);
+unserialize($foo, [
+  'allowed_classes' => FALSE
+]);
+unserialize($foo, array('allowed_classes' => FALSE));
+unserialize($foo, ['allowed_classes' => ['Foo']]);
+unserialize($foo, ['allowed_classes' => ['Foo', 'Bar']]);
+
+// Insecure usages of unserialize should all have one error.
+unserialize($foo, ['allowed_classes' => TRUE]);
+unserialize($foo, [
+  'foo' => 'bar',
+]
+);
+unserialize($foo);
+unserialize(
+  $foo
+);
+unserialize($foo, ['allowed_classes']);
+
+// This is safe but the sniff isn't smart enough to figure it out.
+$allowed = ['allowed_classes' => FALSE];
+unserialize($foo, $allowed);

tests/DrupalPractice/FunctionCalls/InsecureUnserializeUnitTest.php

+<?php
+/**
+ * Unit test class for the InsecureUnserialize sniff.
+ */
+
+namespace DrupalPractice\Test\FunctionCalls;
+
+use Drupal\Test\CoderSniffUnitTest;
+
+/**
+ * Unit test class for the CheckPlain sniff.
+ *
+ * A sniff unit test checks a .inc file for expected violations of a single
+ * coding standard. Expected errors and warnings are stored in this class.
+ */
+class InsecureUnserializeUnitTest extends CoderSniffUnitTest
+{
+
+
+    /**
+     * Returns the lines where errors should occur.
+     *
+     * The key of the array should represent the line number and the value
+     * should represent the number of errors that should occur on that line.
+     *
+     * @param string $testFile The name of the file being tested.
+     *
+     * @return array<int, int>
+     */
+    protected function getErrorList(string $testFile): array
+    {
+        return [
+            13 => 1,
+            15 => 1,
+            18 => 1,
+            21 => 1,
+            22 => 1,
+            26 => 1,
+        ];
+
+    }//end getErrorList()
+
+
+    /**
+     * Returns the lines where warnings should occur.
+     *
+     * The key of the array should represent the line number and the value
+     * should represent the number of warnings that should occur on that line.
+     *
+     * @param string $testFile The name of the file being tested.
+     *
+     * @return array<int, int>
+     */
+    protected function getWarningList(string $testFile): array
+    {
+        return [];
+
+    }//end getWarningList()
+
+
+}//end class