presidentbeef / brakeman

A static analysis security vulnerability scanner for Ruby on Rails applications
https://brakemanscanner.org/
Other
6.97k stars 725 forks source link

Hangs on CheckSQL, CheckRender, CheckExecute without any output of what's going on #19

Closed jmccaffrey closed 12 years ago

jmccaffrey commented 12 years ago

I have a rails 2.3.8 project that I wanted to run brakeman against. I would hang when it hit CheckSQL, and I went through and excluded all of the checks that it would hang on.

For me 'Hang' means it stops at CheckSQL, and does not print anything out for more than 45 mins (at which point I kill it), while it is still consuming mem and cpu.

I tried to get some log info, or see what it was doing via the verbose debug flag, but I couldn't figure out why it was hanging. I imagine it might be some crazy references in my project, if it is actually attempting to look that up, but I'd like some way of knowing what it is getting stuck on.

Is there some other flag to have it tell me what it is doing in those steps?

presidentbeef commented 12 years ago

Hi John,

Thanks for the report. The short answer is no, at this point there are no special flags that will help see what is going on.

What I can easily do is add in more output to indicate what the checks are doing (when the debug flag is set).

Coincidentally, I've been looking at CheckSQL the last couple days. The main issue seems to be that it searches the code three times for calls that may contain SQL. I've been thinking of a way to pre-generate an index of call sites, which would speed up most of these checks.

I assume this is a pretty large Rails app? It may just be taking a long time. As a comparison, I've been running Brakeman against Redmine to clean up some Brakeman issues and it only takes ~1 minute to run CheckSQL.

If it is a smaller Rails app, then I would suspect something is wrong :(

jmccaffrey commented 12 years ago

Good to know that I didn't miss any obvious flags or version constraints. I don't think you have to update the actual gem just because of my issue, I'm sure you have bigger fish to fry.

I've let it go for a very long time, maybe as much as 2 hours, I'm not quite sure. Perhaps its scanning the vendor directory? (which is large, and I wouldn't want it to do)

I can add in some statements to my local version of the gem, and report back to you what I find (now that I know there isn't already some better mechanism or other reason that I am seeing this problem).

I just need to figure out where the best place to insert some logging would be.

presidentbeef commented 12 years ago

You can try --skip-libs, but I don't think it will make a difference. The vendor directory is not scanned in either case.

I would start by looking in lib/checks/check_sql.rb and seeing if the calls to tracker.find_call and tracker.find_model_find are the culprits.

presidentbeef commented 12 years ago

Hi John, please try the 1.0.rc1 release and see if it helps your situation. Thanks!

presidentbeef commented 12 years ago

Just want to check in and see if you have tried the 1.0 release and if it helped at all. Thanks!

presidentbeef commented 12 years ago

Actually, you might want to try the 1.1 release and the --faster option.

jmccaffrey commented 12 years ago

I've tried with the 1.0 version and the 1.1 version, and it still seems to hang. I'm running "brakeman -d --faster" and the last output is for checking for 'indirect XSS'.

I notice it repeats itself:

Checking orders/unsubmitted for direct XSS Checking orders/unsubmitted for indirect XSS Checking orders/unsubmitted for direct XSS Checking orders/unsubmitted for indirect XSS Checking orders/unsubmitted for direct XSS Checking orders/unsubmitted for indirect XSS Checking orders/unsubmitted for direct XSS Checking orders/unsubmitted for indirect XSS Checking orders/unsubmitted for direct XSS Checking orders/unsubmitted for indirect XSS Checking orders/unsubmitted for direct XSS Checking orders/unsubmitted for indirect XSS Checking orders/unsubmitted for direct XSS Checking orders/unsubmitted for indirect XSS Checking orders/unsubmitted for direct XSS Checking orders/unsubmitted for indirect XSS Checking orders/unsubmitted for direct XSS Checking orders/unsubmitted for indirect XSS Checking orders/unsubmitted for direct XSS Checking orders/unsubmitted for indirect XSS

It might not mean its hanging on that step, but is simply the last thing it did before its next huge effort, and the next thing is taking longer than 40mins to print any output.

This is a rails 2.3.8 app, with the rails_xss plugin. I have used the --escape-html flag, and still get the same result.

All I really want is to have it give me some indication of what it is hanging on, so I can look into the problem. I am quite sure its my code that is at fault (I've had to patch RailRoad, and ambitious_query_indexer as a result of goofiness in my code base). I'll keep looking into it, and add a few more debug statements to see what its up to.

presidentbeef commented 12 years ago

Hmmm, that is strange.

Have you tried "-t CheckXSS" to see if it is hanging on that check? Also, if you use "-n" it will do the checks sequentially so it should be easier to see where the problem is.

Sorry the newer versions have not fixed this. Thanks again for the feedback.

On Dec 28, 2011, at 8:28 AM, John McCaffrey reply@reply.github.com wrote:

I've tried with the 1.0 version and the 1.1 version, and it still seems to hang. I'm running "brakeman -d --faster" and the last output is for checking for 'indirect XSS'.

I notice it repeats itself:

Checking orders/unsubmitted for direct XSS Checking orders/unsubmitted for indirect XSS Checking orders/unsubmitted for direct XSS Checking orders/unsubmitted for indirect XSS Checking orders/unsubmitted for direct XSS Checking orders/unsubmitted for indirect XSS Checking orders/unsubmitted for direct XSS Checking orders/unsubmitted for indirect XSS Checking orders/unsubmitted for direct XSS Checking orders/unsubmitted for indirect XSS Checking orders/unsubmitted for direct XSS Checking orders/unsubmitted for indirect XSS Checking orders/unsubmitted for direct XSS Checking orders/unsubmitted for indirect XSS Checking orders/unsubmitted for direct XSS Checking orders/unsubmitted for indirect XSS Checking orders/unsubmitted for direct XSS Checking orders/unsubmitted for indirect XSS Checking orders/unsubmitted for direct XSS Checking orders/unsubmitted for indirect XSS

It might not mean its hanging on that step, but is simply the last thing it did before its next huge effort, and the next thing is taking longer than 40mins to print any output.

This is a rails 2.3.8 app, with the rails_xss plugin. I have used the --escape-html flag, and still get the same result.

All I really want is to have it give me some indication of what it is hanging on, so I can look into the problem. I am quite sure its my code that is at fault (I've had to patch RailRoad, and ambitious_query_indexer as a result of goofiness in my code base). I'll keep looking into it, and add a few more debug statements to see what its up to.


Reply to this email directly or view it on GitHub: https://github.com/presidentbeef/brakeman/issues/19#issuecomment-3292360

presidentbeef commented 12 years ago

Sorry, I mean -t CheckCrossSiteScripting. This will only run that check. You could also try -x CheckCrossSiteScripting to skip that check (or any other check).

xsrgx commented 12 years ago

We have the same problem but only on CheckRender, CheckExecute chekings. I run with options '-t CheckRender --faster' and it hang on. Then run with '-t CheckExecute --faster' with the same result. How can I provide you with more information about these errors?

P.S. On CheckRender it prints "Checking all method bodies for calls to render()" and hang on.

presidentbeef commented 12 years ago

@xsrgx Hm, the question is if it is really hanging or just being really slow.

If you can, please try adding another line of debug output to lib/brakeman/checks/check_render.rb so you can see if it is hanging on a specific method:

    tracker.each_method do |src, class_name, method_name|
      debug_info "Checking class: #{class_name} method: #{method_name}"  #Add this line
      @current_class = class_name
      @current_method = method_name
      process src
    end

If it is hanging on a particular method, I would love to see the source of it if possible. CheckRender is so simple, if it is exhibiting misbehavior it may allow me to fix the trouble with CheckCrossSiteScripting easier.

Otherwise, the upcoming 1.2 release includes changes to CheckRender and CheckExecute which will probably fix the issues you are seeing.

Thanks for reporting this!

presidentbeef commented 12 years ago

Hi John and xsrgx,

Please try the latest release (1.2). I hope it helps!

xsrgx commented 12 years ago

Thank you! Great job! Now it works pretty nice ;)

jmccaffrey commented 12 years ago

awesome!!!!! the latest version works for me as well. Nice work! Thanks for putting your time and effort into this, I really appreciate it. I'm on a project that had an outside firm do a security analysis of our codebase, and I want to see which of the items that they found are also found by brakeman, and what it would take to create new 'checks' to fill in the gaps (so we can make sure that new security issues don't creep in, and to help make brakeman better). Now that all the brakeman checks run on the project, I can do a full side-by-side comparison.

jmccaffrey commented 12 years ago

Closing this issue now, as it is working perfectly for me.

presidentbeef commented 12 years ago

Great! I'm glad we could finally resolve this.

I'd be very interested in the results of your comparison. Please feel free to open issues with suggestions for new checks.