Merge pull request #510 from MauricioFauth/fuzz-errors

Fix undefined array key errors and a deprecation
phpmyadmin · Sep 16, 2023 · 445804d · 445804d
2 parents 3d0c3a6 + dd1e775
commit 445804d
Show file tree

Hide file tree

Showing 23 changed files with 708 additions and 58 deletions.
diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon
@@ -15,11 +15,6 @@ parameters:
  count: 1
  path: src/Components/AlterOperation.php
 
- -
- message: "#^Parameter \\#1 \\$key of function array_key_exists expects int\\|string, float\\|int\\|string given\\.$#"
- count: 2
- path: src/Components/AlterOperation.php
-
  -
  message: "#^Property PhpMyAdmin\\\\SqlParser\\\\Components\\\\AlterOperation\\:\\:\\$options \\(PhpMyAdmin\\\\SqlParser\\\\Components\\\\OptionsArray\\) does not accept PhpMyAdmin\\\\SqlParser\\\\Components\\\\OptionsArray\\|null\\.$#"
  count: 1

diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -12,9 +12,6 @@
  <InvalidScalarArgument occurrences="1">
  <code>$arrayKey</code>
  </InvalidScalarArgument>
- <MixedArrayOffset occurrences="1">
- <code>Parser::$STATEMENT_PARSERS[$token-&gt;value]</code>
- </MixedArrayOffset>
  <MoreSpecificImplementedParamType occurrences="1">
  <code>$component</code>
  </MoreSpecificImplementedParamType>
@@ -631,8 +628,7 @@
  <code>$this-&gt;last</code>
  <code>$this-&gt;last</code>
  </LoopInvalidation>
- <MixedArrayAccess occurrences="42">
- <code>$this-&gt;str[$this-&gt;last + 1]</code>
+ <MixedArrayAccess occurrences="41">
  <code>$this-&gt;str[$this-&gt;last + 1]</code>
  <code>$this-&gt;str[$this-&gt;last++]</code>
  <code>$this-&gt;str[$this-&gt;last]</code>

diff --git a/src/Components/AlterOperation.php b/src/Components/AlterOperation.php
@@ -11,7 +11,7 @@
 
 use function array_key_exists;
 use function in_array;
-use function is_numeric;
+use function is_int;
 use function is_string;
 use function trim;
 
@@ -412,7 +412,7 @@ public static function parse(Parser $parser, TokensList $list, array $options =
 
  $state = 2;
  } elseif ($state === 2) {
- if (is_string($token->value) || is_numeric($token->value)) {
+ if (is_string($token->value) || is_int($token->value)) {
  $arrayKey = $token->value;
  } else {
  $arrayKey = $token->token;
@@ -445,7 +445,7 @@ public static function parse(Parser $parser, TokensList $list, array $options =
  );
  break;
  }
- } elseif (! empty(Parser::$STATEMENT_PARSERS[$token->value])) {
+ } elseif (! empty(Parser::$STATEMENT_PARSERS[$arrayKey])) {
  // We have reached the end of ALTER operation and suddenly found
  // a start to new statement, but have not found a delimiter between them
  $parser->error(

diff --git a/src/Components/ArrayObj.php b/src/Components/ArrayObj.php
@@ -92,6 +92,10 @@ public static function parse(Parser $parser, TokensList $list, array $options =
 
  // End of statement.
  if ($token->type === Token::TYPE_DELIMITER) {
+ if ($brackets > 0) {
+ $parser->error('A closing bracket was expected.', $token);
+ }
+
  break;
  }
 

diff --git a/src/Statements/AlterStatement.php b/src/Statements/AlterStatement.php
@@ -67,7 +67,14 @@ class AlterStatement extends Statement
  public function parse(Parser $parser, TokensList $list)
  {
  ++$list->idx; // Skipping `ALTER`.
- $this->options = OptionsArray::parse($parser, $list, static::$OPTIONS);
+ $parsedOptions = OptionsArray::parse($parser, $list, static::$OPTIONS);
+ if ($parsedOptions->isEmpty()) {
+ $parser->error('Unrecognized alter operation.', $list->tokens[$list->idx]);
+
+ return;
+ }
+
+ $this->options = $parsedOptions;
  ++$list->idx;
 
  // Parsing affected table.

diff --git a/src/Statements/WithStatement.php b/src/Statements/WithStatement.php
@@ -16,6 +16,7 @@
 
 use function array_slice;
 use function count;
+use function preg_match;
 
 /**
  * `WITH` statement.
@@ -114,7 +115,7 @@ public function parse(Parser $parser, TokensList $list)
  }
 
  if ($state === 0) {
- if ($token->type !== Token::TYPE_NONE) {
+ if ($token->type !== Token::TYPE_NONE || ! preg_match('/^[a-zA-Z0-9_$]+$/', $token->token)) {
  $parser->error('The name of the CTE was expected.', $token);
  break;
  }
@@ -124,7 +125,12 @@ public function parse(Parser $parser, TokensList $list)
  $state = 1;
  } elseif ($state === 1) {
  if ($token->type === Token::TYPE_OPERATOR && $token->value === '(') {
- $this->withers[$wither]->columns = Array2d::parse($parser, $list);
+ $columns = Array2d::parse($parser, $list);
+ if ($parser->errors !== []) {
+ break;
+ }
+
+ $this->withers[$wither]->columns = $columns;
  $state = 2;
  } elseif ($token->type === Token::TYPE_KEYWORD && $token->keyword === 'AS') {
  $state = 3;

diff --git a/src/Token.php b/src/Token.php
@@ -254,8 +254,8 @@ public function extract()
  case self::TYPE_NUMBER:
  $ret = str_replace('--', '', $this->token); // e.g. ---42 === -42
  if ($this->flags & self::FLAG_NUMBER_HEX) {
+ $ret = str_replace(['-', '+'], '', $this->token);
  if ($this->flags & self::FLAG_NUMBER_NEGATIVE) {
- $ret = str_replace('-', '', $this->token);
  $ret = -hexdec($ret);
  } else {
  $ret = hexdec($ret);

diff --git a/tests/Misc/BugsTest.php b/tests/Misc/BugsTest.php
@@ -22,6 +22,10 @@ public function testBug(string $test): void
  public function bugProvider(): array
  {
  return [
+ ['bugs/fuzz1'],
+ ['bugs/fuzz2'],
+ ['bugs/fuzz3'],
+ ['bugs/fuzz4'],
  ['bugs/gh9'],
  ['bugs/gh14'],
  ['bugs/gh16'],

diff --git a/tests/data/bugs/fuzz1.in b/tests/data/bugs/fuzz1.in
@@ -0,0 +1 @@
+ALTER..2
diff --git a/tests/data/bugs/fuzz1.out b/tests/data/bugs/fuzz1.out
@@ -0,0 +1,94 @@
+{
+ "query": "ALTER..2",
+ "lexer": {
+ "@type": "PhpMyAdmin\\SqlParser\\Lexer",
+ "str": "ALTER..2",
+ "len": 8,
+ "last": 8,
+ "list": {
+ "@type": "PhpMyAdmin\\SqlParser\\TokensList",
+ "tokens": [
+ {
+ "@type": "PhpMyAdmin\\SqlParser\\Token",
+ "token": "ALTER",
+ "value": "ALTER",
+ "keyword": "ALTER",
+ "type": 1,
+ "flags": 3,
+ "position": 0
+ },
+ {
+ "@type": "PhpMyAdmin\\SqlParser\\Token",
+ "token": ".",
+ "value": ".",
+ "keyword": null,
+ "type": 2,
+ "flags": 16,
+ "position": 5
+ },
+ {
+ "@type": "PhpMyAdmin\\SqlParser\\Token",
+ "token": ".2",
+ "value": 0.2,
+ "keyword": null,
+ "type": 6,
+ "flags": 2,
+ "position": 6
+ },
+ {
+ "@type": "PhpMyAdmin\\SqlParser\\Token",
+ "token": null,
+ "value": null,
+ "keyword": null,
+ "type": 9,
+ "flags": 0,
+ "position": null
+ }
+ ],
+ "count": 4,
+ "idx": 4
+ },
+ "delimiter": ";",
+ "delimiterLen": 1,
+ "strict": false,
+ "errors": []
+ },
+ "parser": {
+ "@type": "PhpMyAdmin\\SqlParser\\Parser",
+ "list": {
+ "@type": "@1"
+ },
+ "statements": [
+ {
+ "@type": "PhpMyAdmin\\SqlParser\\Statements\\AlterStatement",
+ "table": null,
+ "altered": [],
+ "options": null,
+ "first": 0,
+ "last": 0
+ }
+ ],
+ "brackets": 0,
+ "strict": false,
+ "errors": []
+ },
+ "errors": {
+ "lexer": [],
+ "parser": [
+ [
+ "Unrecognized alter operation.",
+ {
+ "@type": "@2"
+ },
+ 0
+ ],
+ [
+ "Unexpected beginning of statement.",
+ {
+ "@type": "@4"
+ },
+ 0
+ ]
+ ]
+ }
+}
diff --git a/tests/data/bugs/fuzz2.in b/tests/data/bugs/fuzz2.in
@@ -0,0 +1 @@
+WITH](
diff --git a/tests/data/bugs/fuzz2.out b/tests/data/bugs/fuzz2.out
@@ -0,0 +1,111 @@
+{
+ "query": "WITH](",
+ "lexer": {
+ "@type": "PhpMyAdmin\\SqlParser\\Lexer",
+ "str": "WITH](",
+ "len": 6,
+ "last": 6,
+ "list": {
+ "@type": "PhpMyAdmin\\SqlParser\\TokensList",
+ "tokens": [
+ {
+ "@type": "PhpMyAdmin\\SqlParser\\Token",
+ "token": "WITH",
+ "value": "WITH",
+ "keyword": "WITH",
+ "type": 1,
+ "flags": 3,
+ "position": 0
+ },
+ {
+ "@type": "PhpMyAdmin\\SqlParser\\Token",
+ "token": "]",
+ "value": "]",
+ "keyword": null,
+ "type": 0,
+ "flags": 0,
+ "position": 4
+ },
+ {
+ "@type": "PhpMyAdmin\\SqlParser\\Token",
+ "token": "(",
+ "value": "(",
+ "keyword": null,
+ "type": 2,
+ "flags": 16,
+ "position": 5
+ },
+ {
+ "@type": "PhpMyAdmin\\SqlParser\\Token",
+ "token": null,
+ "value": null,
+ "keyword": null,
+ "type": 9,
+ "flags": 0,
+ "position": null
+ }
+ ],
+ "count": 4,
+ "idx": 4
+ },
+ "delimiter": ";",
+ "delimiterLen": 1,
+ "strict": false,
+ "errors": []
+ },
+ "parser": {
+ "@type": "PhpMyAdmin\\SqlParser\\Parser",
+ "list": {
+ "@type": "@1"
+ },
+ "statements": [
+ {
+ "@type": "PhpMyAdmin\\SqlParser\\Statements\\WithStatement",
+ "withers": [],
+ "cteStatementParser": null,
+ "options": {
+ "@type": "PhpMyAdmin\\SqlParser\\Components\\OptionsArray",
+ "options": []
+ },
+ "first": 0,
+ "last": 0
+ }
+ ],
+ "brackets": 1,
+ "strict": false,
+ "errors": []
+ },
+ "errors": {
+ "lexer": [
+ [
+ "Unexpected character.",
+ "]",
+ 4,
+ 0
+ ]
+ ],
+ "parser": [
+ [
+ "The name of the CTE was expected.",
+ {
+ "@type": "@3"
+ },
+ 0
+ ],
+ [
+ "Unexpected end of the WITH CTE.",
+ {
+ "@type": "@3"
+ },
+ 0
+ ],
+ [
+ "Unexpected beginning of statement.",
+ {
+ "@type": "@3"
+ },
+ 0
+ ]
+ ]
+ }
+}
diff --git a/tests/data/bugs/fuzz3.in b/tests/data/bugs/fuzz3.in
@@ -0,0 +1 @@
+WITH*/A(