Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug: SQL injection works with heredoc, but not with nowdoc #243

Closed
2 tasks done
greg0ire opened this issue May 17, 2024 · 2 comments · Fixed by #244
Closed
2 tasks done

bug: SQL injection works with heredoc, but not with nowdoc #243

greg0ire opened this issue May 17, 2024 · 2 comments · Fixed by #244
Labels

Comments

@greg0ire
Copy link

greg0ire commented May 17, 2024

Did you check existing issues?

  • I have read all the tree-sitter docs if it relates to using the parser
  • I have searched the existing issues of tree-sitter-php

Tree-Sitter CLI Version, if relevant (output of tree-sitter --version)

tree-sitter 0.22.5

Describe the bug

SQL is not recognized as such when enclosed in nowdoc.

Steps To Reproduce/Bad Parse Tree

      right: (nowdoc) ; [130:16 - 135:3]
       identifier: (heredoc_start) ; [130:20 - 22]
       value: (nowdoc_body) ; [130:24 - 134:1]
        (nowdoc_string) ; [131:1 - 24]
        (nowdoc_string) ; [131:25 - 132:22]
        (nowdoc_string) ; [132:23 - 133:18]
        (nowdoc_string) ; [133:19 - 134:1]
       end_tag: (heredoc_end) ; [134:2 - 135:3]

Expected Behavior/Parse Tree

      right: (nowdoc) ; [130:16 - 135:3]
       identifier: (nowdoc_start) ; [130:20 - 22]
       value: (nowdoc_body) ; [130:24 - 134:1]
        (statement) ; [131:1 - 133:18]
         (create_table) ; [131:1 - 133:18]
          (keyword_create) ; [131:1 - 6]
          (keyword_table) ; [131:8 - 12]
          (object_reference) ; [131:14 - 22]
           name: (identifier) ; [131:14 - 22]
          (column_definitions) ; [131:24 - 133:18]
           (column_definition) ; [132:5 - 20]
            name: (identifier) ; [132:5 - 7]
            type: (varchar) ; [132:9 - 20]
             (keyword_varchar) ; [132:9 - 15]
             size: (literal) ; [132:18 - 19]
           (column_definition) ; [133:5 - 12]
            name: (identifier) ; [133:5 - 7]
            type: (keyword_text) ; [133:9 - 12]
           (keyword_having) ; [133:14 - 17]
        (keyword_having) ; [134:1 - 1]
        (string_content) ; [131:1 - 24]
        (string_content) ; [132:1 - 22]
        (string_content) ; [133:1 - 18]
        (string_content) ; [134:1 - 1]
       end_tag: (nowdoc_end) ; [135:1 - 3]

Repro

// Example code that causes the issue
function foo() {
  // Code that fails to parse, or causes an error
        $sql = <<<'SQL'
CREATE TABLE dbal_1779 (
    foo VARCHAR (64) ,
    bar TEXT (100)
)
SQL;
}

Maybe the issue is that the tree shows heredoc_start and not nowdoc_start, if such a thing exists…

@greg0ire greg0ire added the bug label May 17, 2024
@calebdw
Copy link
Collaborator

calebdw commented May 17, 2024

Maybe the issue is that the tree shows heredoc_start and not nowdoc_start

No that's not the issue, the Nowdoc reuses the same heredoc_{start,end} nodes

@calebdw
Copy link
Collaborator

calebdw commented May 17, 2024

Issue is that the heredoc_end is sometimes including the whitespace (for nowdocs only):

image

However, does not seem to do this if there's a comma:

image

@greg0ire greg0ire changed the title bug: SQL injection works with heredoc, but now with nowdoc bug: SQL injection works with heredoc, but not with nowdoc May 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants