infobyte / faraday_plugins

Security tools report parsers for Faradaysec.com
https://www.faradaysec.com/
GNU General Public License v3.0
47 stars 17 forks source link

Bandit Plugin #3

Closed flopezluksenberg closed 3 years ago

aenima-x commented 3 years ago

Hello @flopezluksenberg can you give us a xml sample for this plugin

flopezluksenberg commented 3 years ago

Hi @aenima-x !

I've attached a Zip file that contains inside the XML report. I've used this repository (over the bad folder) to execute the following command:

bandit --exclude ./venv -r . -f xml -o vulpy_faraday_bandit.xml

A simple note about this report plugin: Bandit is a code analyzer, so there is no host available. I've used the filename as host IP (defaults to 'bandit-report' if something fails) because it accepts a simple string. Let me know if that isn't correct, or what you think I can change knowing this limitation.

vulpy_faraday_bandit.xml.zip

aenima-x commented 3 years ago

Yes, that is the main issue. We have it currently on our roadmap to add code vulns. But is not ready, I think we will put your PR on hold until that

llazzaro commented 3 years ago

you can try to execute:

bandit -f xml -o test.xml -r -v .

On any python repository

EricHorvat commented 3 years ago

Hi @flopezluksenberg, we want to merge this PR, but it needs some improves.

As for now, the name detection its ok, but when using the dection of test.xml as @llazzaro said, it will not work.

We encourage you to read the XMLPlugin class, the report_belongs_to function, which will use the field for that. Seeing the output of the xml as you said to be run, the main tag is <testsuite name="bandit" tests="X">, so you probably have to override the report_belongs_to function, reusing it with testsuite identifier tag, and then check that the tag has name="bandit" in it.

If you have any doubt, don't hesitate in reply in this PR.

flopezluksenberg commented 3 years ago

Hi Guys! Sorry for the really late answer!

@EricHorvat I've added the report_belongs_to bandit recognition, now it can be recognized without using the name in the file. As you mentioned, I've modified the identifier_tag from bandit to testsuite.

Let me know if you need something more!

aenima-x commented 3 years ago

@flopezluksenberg A question, it would not make more sense to use each file as host? because in this case being code vulnerabilities each affected file would be the asset. And could have more than 1 vulnerability.

flopezluksenberg commented 3 years ago

I've decided to use the filename as host because it could be easy identified in Faraday (at least as a workaround of not supports host from code analyzers like bandit), but if you guys think that is better to have a host per analyzed file, I can make that change.

I remember that there is another code analyzer plugin available in faraday but I can't remember which. Do you know what plugin it is and how the host is defined in that plugin?

aenima-x commented 3 years ago

@flopezluksenberg If you could do that I think it will be better. Because it would be a better analogy treat each file as a host and add all of its vulns to it.

No, I dont remember other code plugin

llazzaro commented 3 years ago

I think you are looking for this -> https://github.com/infobyte/faraday_plugins/blob/f744ebb6673e5a2ad6f2c9847253c1bea18ac468/faraday_plugins/plugins/repo/appscan/plugin.py#L341

aenima-x commented 3 years ago

Thanks @llazzaro See @flopezluksenberg in appscan we create a host for each source file with vulns.

flopezluksenberg commented 3 years ago

@aenima-x I've changed the host name of vulns. Now the host name is picked from the path of the vuln instead of using a harcoded name for all vulns as before.

image This is an example of how it looks in the workspace dashboard after uploading the report.

Let me know if is required to make another change or tweak this approach 😃

aenima-x commented 3 years ago

@flopezluksenberg Great, push those changes and we will merge the PR

flopezluksenberg commented 3 years ago

I've pushed it! look at line 30 of this file: faraday_plugins/plugins/repo/bandit/plugin.py

aenima-x commented 3 years ago

@flopezluksenberg ok so this could be deleted?

   def _get_host_name(self):
         try:
             filename = self.vulns_data['command']['params'].split('/')[-1].lower()
             if filename.endswith('_faraday_bandit.xml'):
                 return filename.lower().replace('_faraday_bandit.xml', '')

             return filename
         except:
             pass

         return 'bandit-report'
flopezluksenberg commented 3 years ago

you're right! sorry for that, I will let you know when I've deleted it!

flopezluksenberg commented 3 years ago

Hi @aenima-x ! Sorry for the late answer! I've deleted the unused method _get_host_name of BanditPlugin.

aenima-x commented 3 years ago

@flopezluksenberg I will ask you one last thing. The recently made a change, please update you fork and make this change

def createPlugin(ignore_info=False):
     return BanditPlugin(ignore_info=ignore_info)

And this in the Plugin init

    def __init__(self, *arg, **kwargs):
        super().__init__(*arg, **kwargs)
flopezluksenberg commented 3 years ago

@aenima-x sorry, my fork was outdated! I have already updated it and I have made the required changes you asked me. Let me know if you need anything more.