🐛 Fix Defender broken Endpoint #11217

[manuel-sommer]

Defender can be used (rolled out) on various devices: servers, mobile phones, IoT devices, clients. This results in broken endpoints, because DefectDojo parses "computerDNSName" as an endpoint. However, dropping this information is not beneficial as it is an important identifier especially for mobile phones, IoT devices and clients. The problem is that the computerDNSName of these devices do not always follow https://en.wikipedia.org/wiki/URL#Syntax.
Thus, this information needs to be cleaned up. An example is the following string:
Max Mustermann iPadAir 17zoll (2ndgeneration)

[dryrunsecurity]

DryRun Security Summary

The provided code changes focus on improving the unit tests for the MSDefenderParser class, including the addition of a new test case to validate the parser's handling of a specific issue (issue #11217) related to parsing Microsoft Defender scan reports, and addressing the formatting of the computerDnsName field from the machine object in the dojo/tools/ms_defender/parser.py file.

Expand for full summary

Summary:

The provided code changes are focused on improving the unit tests for the MSDefenderParser class, which is part of the application's security tooling. The changes include the addition of a new test case to validate the parser's handling of a specific issue (issue #11217) related to parsing Microsoft Defender scan reports.

The new test case ensures that the parser correctly processes the scan report, identifies the expected finding, and sets the correct severity and host value for the unsaved endpoint. This addition helps to improve the reliability and robustness of the MSDefenderParser class by verifying its behavior in a specific scenario.

Additionally, the changes in the dojo/tools/ms_defender/parser.py file address the formatting of the computerDnsName field from the machine object. The code performs various string transformations to ensure that the Endpoint object is properly constructed and can be saved without any issues. This change does not introduce any significant security concerns, but it's important to review the overall context of the code to ensure that there are no other potential security vulnerabilities or concerns.

Files Changed:

1. unittests/tools/test_ms_defender_parser.py:

   • A new test case, test_parser_defender_issue_11217, has been added to validate the parser's handling of a specific issue (issue MS Defender does not parse Endpoints right #11217) related to parsing Microsoft Defender scan reports.
   • The new test case checks that the parser correctly processes the scan report, identifies the expected finding, and sets the correct severity and host value for the unsaved endpoint.

2. dojo/tools/ms_defender/parser.py:

   • The code changes in this file address the formatting of the computerDnsName field from the machine object in the process_zip method of the MSDefenderParser class.
   • The changes involve performing string transformations to remove spaces, replace open and close parentheses with underscores, to ensure that the Endpoint object is properly constructed and can be saved without any issues.

Code Analysis

We ran 9 analyzers against 3 files and 0 analyzers had findings. 9 analyzers had no findings.

Riskiness

🟢 Risk threshold not exceeded.

View PR in the DryRun Dashboard.

[kiblik]

I'm not sure about putting DNS name to UserInfo. It is quite well defined, what should be there

An optional userinfo subcomponent followed by an at symbol (@), that may consist of a user name and an optional password preceded by a colon (:). Use of the format username:password in the userinfo subcomponent is deprecated for security reasons. Applications should not render as clear text any data after the first colon (:) found within a userinfo subcomponent unless the data after the colon is the empty string (indicating no password).

https://en.wikipedia.org/wiki/URL#Syntax

[cneill]

Would you mind adding one or more unit test cases that evaluate the behavior you're seeing in the wild @manuel-sommer ? I'm surprised space, (, and ) characters are ever showing up in real "DNS names". I wonder if these are really NetBIOS names or something, and if we should perhaps distinguish them from real DNS entries somehow rather than forcing them to conform to DNS structure here by just replacing those characters.

[manuel-sommer]

#11217

[mtesauro]

Approved

[manuel-sommer]

Could you merge this @mtesauro?