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

Parse error with rails4 option #1059

Closed
kaizencodes opened this Issue Jun 19, 2017 · 7 comments

Comments

Projects
None yet
2 participants
@kaizencodes

kaizencodes commented Jun 19, 2017

Using the rails4 option results in parse error on value ":" (tCOLON) for every <%= form_for :something do %>

I've tested out with erubis -x file_name.html.erb | ruby -c

-:19: syntax error, unexpected ')'
...for model do |f| ).to_s; _buf << '

Seems to be a problem with erb. What is strange however, that leaving out the rails4 option makes Brakeman pass.

@presidentbeef

This comment has been minimized.

Show comment
Hide comment
@presidentbeef

presidentbeef Jun 22, 2017

Owner

Yes, Rails does its own ERB stuff and Brakeman tries to match it. Is it actually a Rails 4 application you are scanning?

Owner

presidentbeef commented Jun 22, 2017

Yes, Rails does its own ERB stuff and Brakeman tries to match it. Is it actually a Rails 4 application you are scanning?

@kaizencodes

This comment has been minimized.

Show comment
Hide comment
@kaizencodes

kaizencodes Jun 26, 2017

Yes it is 4.2.8.

Are there any difference in how brakeman runs with rails4 option vs auto detect?

kaizencodes commented Jun 26, 2017

Yes it is 4.2.8.

Are there any difference in how brakeman runs with rails4 option vs auto detect?

@presidentbeef

This comment has been minimized.

Show comment
Hide comment
@presidentbeef

presidentbeef Jun 26, 2017

Owner

You saw this in Brakeman's output?

[Notice] Detected Rails 4 application

and what was the version reported as in the Brakeman report?

Owner

presidentbeef commented Jun 26, 2017

You saw this in Brakeman's output?

[Notice] Detected Rails 4 application

and what was the version reported as in the Brakeman report?

@kaizencodes

This comment has been minimized.

Show comment
Hide comment
@kaizencodes

kaizencodes Jun 27, 2017

[Notice] Detected Rails 4 application yes

"rails_version": "4.2.8",

kaizencodes commented Jun 27, 2017

[Notice] Detected Rails 4 application yes

"rails_version": "4.2.8",

@presidentbeef

This comment has been minimized.

Show comment
Hide comment
@presidentbeef

presidentbeef Jun 28, 2017

Owner

Pretty weird... I'm not seeing any reason why that would happen. As far as I can tell there is no difference between using --rails4 or the auto-detect (and there shouldn't be).

You are also seeing

[Notice] Escaping HTML by default

right?

I can't reproduce the issue, either. Can you provide a more complete ERB template to reproduce?

I don't suppose this is an open source application?

Owner

presidentbeef commented Jun 28, 2017

Pretty weird... I'm not seeing any reason why that would happen. As far as I can tell there is no difference between using --rails4 or the auto-detect (and there shouldn't be).

You are also seeing

[Notice] Escaping HTML by default

right?

I can't reproduce the issue, either. Can you provide a more complete ERB template to reproduce?

I don't suppose this is an open source application?

@kaizencodes

This comment has been minimized.

Show comment
Hide comment
@kaizencodes

kaizencodes Jun 28, 2017

It's closed source yes. but here is a simple reproduction on the same rails version repo.
running brakeman as is will cause the parsing error, commenting out https://github.com/kaizencodes/brakeman_test/blob/master/config/brakeman.yml#L1 will make it pass.

I do see [Notice] Escaping HTML by default when it's autodetecting, however not when :rails4: true

kaizencodes commented Jun 28, 2017

It's closed source yes. but here is a simple reproduction on the same rails version repo.
running brakeman as is will cause the parsing error, commenting out https://github.com/kaizencodes/brakeman_test/blob/master/config/brakeman.yml#L1 will make it pass.

I do see [Notice] Escaping HTML by default when it's autodetecting, however not when :rails4: true

presidentbeef added a commit that referenced this issue Jun 29, 2017

@presidentbeef

This comment has been minimized.

Show comment
Hide comment
@presidentbeef

presidentbeef Jun 29, 2017

Owner

AH okay. The key bit of information was the option was set in a configuration file.

Owner

presidentbeef commented Jun 29, 2017

AH okay. The key bit of information was the option was set in a configuration file.

Repository owner locked and limited conversation to collaborators Aug 7, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.