Add basic parser for Outpost24 scan format

[jvz]

@madchap @jeffret-b @Wadeck @daniel-beck

When submitting a pull request, please make sure you have completed the following checklist:

• Your code is flake8 compliant
• Your code is python 3.5 compliant
• If this is a new feature and not a bug fix, you've included the proper documentation in the ReadTheDocs documentation folder. https://github.com/DefectDojo/Documentation/tree/master/docs or provide feature documentation in the PR.
• Model changes must include the necessary migrations in the dojo/dd_migrations folder.
• Add applicable tests to the unit tests.

[Maffooch]

Don't forget about adding the fixture :)

[madchap]

@jvz test_type.json fixture that is ;-)

Also, additional couple tests for 1 finding and 0 finding to follow on what is being done today. The travis build is failing because of flake8, make sure you double-check on that.

Cheers!
fred

[jvz]

Alright, I've addressed those issues. There's still a little bit of code to add for the endpoints part, though I'll follow up with that later today. Let's make sure this part passes CI at least.

[jvz]

Seems to be a test failure still. I'll update later today.

[jvz]

Figured it out. @jeffret-b feel free to review now!

[jeffret-b]

It will be nice to have a parser for this format to import findings when there is a need. I don't have a lot of experience in DefectDojo code, but the proposed changes here all look clean and correct.

[jeffret-b]

Does it make sense to have at least one test that check the finding details?

[jvz]

You'd think so, but I didn't see any other tests doing that. 🤷‍♂

[jeffret-b]

I agree that it doesn't seem to be common in these tests, so maybe it isn't valuable. I did find this example, though, that is doing something along those lines:

django-DefectDojo/dojo/unittests/test_checkmarx_parser.py

Line 165 in ac05024

def check_parse_file_with_single_vulnerability_has_single_finding(self, parser):

[jeffret-b]

Remaining to be done before merge

@jvz is this comment in other original description still accurate? Is there still remaining work to be done?

[madchap]

I think the parsing could add a few items.

For example, the CVSS scores (probably both 2 and 3) and vectors could be added to the "Severity justification" field in DD. Also the <cvss_vector_description> could also be added or eventually appended to the overall description.

Also, the "References" field could eventually benefit from info in the <referencelist>.

What do you think?

[jvz]

When you merge this, please squash the history. Thanks!

[madchap]

Looks good to me, thanks for making these changes!

[madchap]

@jvz if you could upload that sample file as well in the sibling repo at https://github.com/DefectDojo/sample-scan-files, it would be great. Thanks!

[ptrovatelli]

looks pretty neat.
was this sample.xml file manually modified? there are a lot of duplicates for example <id>1377090</id> appearing twice with just firstseen changed
if this is a regular report, it might be good to do some aggregation upon import; it'll greatly improve the performances by inserted much less lines into the db and also will make the analysis easier (or maybe just ignore part of the report if we can find that some part is duplicate with another part)

dependency check amongst other scanners do this, using a dict. ideally update nb_occurences in that case.

it might be nice to use that falsepositive flag too to fill in the according field in the db

[jvz]

was this sample.xml file manually modified? there are a lot of duplicates for example <id>1377090</id> appearing twice with just firstseen changed

Yes, I started from a real scan, removed several things, and didn't really update any summary statistics at the top as I wasn't sure how they're calculated by the tool itself. I'm not sure exactly what the id element corresponds to, but it's a tool-specific id.