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

Command injection missed when backticks have operator #1183

Closed
JacobEvelyn opened this issue Apr 25, 2018 · 4 comments · Fixed by #1185
Closed

Command injection missed when backticks have operator #1183

JacobEvelyn opened this issue Apr 25, 2018 · 4 comments · Fixed by #1185

Comments

@JacobEvelyn
Copy link
Contributor

Background

Brakeman version: 4.2.1
Rails version: 5.1.4
Ruby version: 2.5.1

Issue

Brakeman correctly spots this vulnerability:

def dangerous(path)
  `echo #{path}`
end

But misses it when we change the code to:

def dangerous(path)
  `echo #{path}`.chomp
end

It appears to miss all command injection vulnerabilities in backticks that have a method called on them, regardless of what that method is or does.

@presidentbeef
Copy link
Owner

Thanks, fixing that turned up quite a few missed issues.

@JacobEvelyn
Copy link
Contributor Author

Awesome, glad to hear it! Out of curiosity (not trying to nag), what does your RubyGems release cadence look like? (Don't go out of your way to cut a new release just for us; I'm really just wondering how often we should expect new gem versions in the future.)

@presidentbeef
Copy link
Owner

Funny you should ask. I was just thinking there should be an April release but probably not going to happen. Maybe sometime next week.

BTW, as of today, there have been 106 releases in 2,800 days, so on average a release every 26 days :P

@JacobEvelyn
Copy link
Contributor Author

Great, thanks for the info!

Repository owner locked and limited conversation to collaborators Jul 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants