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.

Sign up for GitHub

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

Jump to bottom

Fixed list parsing #52

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Fixed list parsing #52

wants to merge 4 commits into from

Conversation

DevNeol
Copy link

@DevNeol DevNeol commented Nov 9, 2023 •
edited by MattBudz
Loading

Summary

The function format_list has been implemented to resolve the parsing problems of lists from XML exports of Acunetix.
The previous version was not able to correctly define the depth levels of the elements in a list.

Testing Steps

  1. download https://github.com/DevNeol/dradis-acunetix/blob/main/spec/fixtures/files/testphp.vulnweb.com.export.acunetix.xml
  2. navigate to /projects/<PROJECT_ID>/upload
  3. select acunetix and upload the file from step 1
  4. wait for it to upload
  5. navigate to the evidence associated with Vulnerable JavaScript libraries
  6. assert the Details field content contains a nested list

Other Information

Credits also to: https://github.com/rbctee/

Copyright assignment

I assign all rights, including copyright, to any future Dradis
work by myself to Security Roots.

@MattBudz
Copy link
Contributor

MattBudz commented Dec 13, 2023 •
edited
Loading

@DevNeol thanks for submitting this. Please review the CONTRIBUTING.md, and ensure you include:

  • a CHANGELOG entry
  • specs for your changes
  • testing steps in your pull request's description

We'll update the PR template with a checklist similar to the above to help with future PR submissions.

@MattBudz MattBudz assigned MattBudz and DevNeol and unassigned MattBudz Dec 13, 2023
@DevNeol
Copy link
Author

DevNeol commented Dec 13, 2023

Hi @MattBudz,
CHANGELOG: Fixed the parsing of depth-level elements in a list.
tests for your changes: We used the edited version of the plugin for about 18 months, and all the outputs have been processed properly.
testing steps in your pull request's description: We had some issues with the Acunetix plugin due to Dradis incorrectly parsing the XML output generated by Acunetix. More in particular, when importing the XML document exported by Acunetix, which I attached tbelow, the contents of the “Details” field of an Evidence isn’t parsed correctly.

Follows the output shown by Acunetix (how should be):
immagine

Follows the output shown by Dradis: (as-is)
immagine

Based on what we’ve found, this problems seem to stem from the source code of the Acunetix plugin.
To be more specific, the script cleanup.rb (https://github.com/dradis/dradis-acunetix/blob/main/lib/acunetix/concerns/cleanup.rb) uses the gsub function to replace the HTML elements with Textile syntax.
However, doing so nested HTML elements are ignored, and the HTML isn’t correctly converted to Textile markup.

The function format_list has been implemented to resolve the parsing problems of lists from XML exports of Acunetix.

PS: could you please add also /rbctee/ as partecipants please?

Please, let me know if something is missing.

Thank you

@MattBudz
Copy link
Contributor

Thanks for the extra info @DevNeol

Please, let me know if something is missing.

For the testing steps in the PR description, please add specific step-by-step instructions to test the new functionality. Something like:

  1. Navigate to...
  2. Click on...
  3. Upload file...
  4. Assert...

As for adding tests/specs for your changes, those live in https://github.com/DevNeol/dradis-acunetix/tree/main/spec and the Changelog entry should be added to https://github.com/DevNeol/dradis-acunetix/blob/main/CHANGELOG.md using this template:

[v#.#.#] ([month] [YYYY])
  - [future tense verb] [feature]

the [v#.#.#] ([month] [YYYY]) portion can remain as a placeholder, you just need to add a real entry in place of [future tense verb] [feature]. In this case it could be something like Add support for nested items in lists

@rbctee
Copy link

rbctee commented Dec 21, 2023

Hi there, I'd like to add some more information regarding the fix.
First, to test it I added a new ReportItem element to the XML file in https://github.com/DevNeol/dradis-acunetix/blob/main/spec/fixtures/files/testphp.vulnweb.com.export.acunetix.xml, containing some nested lists.

      <ReportItem id="195" color="orange">
        <Name><![CDATA[Vulnerable JavaScript libraries]]></Name>
        <ModuleName><![CDATA[/deepscan/javascript_library_audit_deepscan.js]]></ModuleName>
        <Details><![CDATA[<ul>
<li><strong>jQuery 1.12.4</strong>
<ul>
<li>URL: http://testphp.vulnweb.com/</li>
<li>Detection method: The library&#x27;s name and version were determined based on its dynamic behavior. </li>
<li>CVE-ID: CVE-2015-9251, CVE-2020-11022, CVE-2020-11023</li>
<li>Description: Possible Cross Site Scripting via third-party text/javascript responses (1.12.0-1.12.2 mitigation reverted) / In jQuery versions greater than or equal to 1.2 and before 3.5.0, passing HTML from untrusted sources - even after sanitizing it - to one of jQuery&#x27;s DOM manipulation methods (i.e. .html(), .append(), and others) may execute untrusted code. This problem is patched in jQuery 3.5.0. / In jQuery versions greater than or equal to 1.0.3 and before 3.5.0, passing HTML containing option elements from untrusted sources - even after sanitizing it - to one of jQuery&#x27;s DOM manipulation methods (i.e. .html(), .append(), and others) may execute untrusted code. This problem is patched in jQuery 3.5.0. </li>
<li>References:
<ul>
<li>https://github.com/jquery/jquery/issues/2432</li>
<li>https://blog.jquery.com/2020/04/10/jquery-3-5-0-released/</li>
<li>https://mksben.l0.cm/2020/05/jquery3.5.0-xss.html</li>
<li>https://jquery.com/upgrade-guide/3.5/</li>
<li>https://api.jquery.com/jQuery.htmlPrefilter/</li>
<li>https://www.cvedetails.com/cve/CVE-2020-11022/</li>
<li>https://github.com/advisories/GHSA-gxr4-xjj5-5px2</li>
<li>https://www.cvedetails.com/cve/CVE-2020-11023/</li>
<li>https://github.com/advisories/GHSA-jpcq-cgw6-v4j6</li>
</ul>
</li>
</ul>
</li></ul>]]></Details>
        <Affects><![CDATA[/]]></Affects>
        <Parameter><![CDATA[]]></Parameter>
        <AOP_SourceFile><![CDATA[]]></AOP_SourceFile>
        <AOP_SourceLine></AOP_SourceLine>
        <AOP_Additional><![CDATA[]]></AOP_Additional>
        <IsFalsePositive><![CDATA[]]></IsFalsePositive>
        <Severity><![CDATA[medium]]></Severity>
        <Type><![CDATA[]]></Type>
        <Impact><![CDATA[Consult References for more information.]]></Impact>
        <Description><![CDATA[You are using one or more vulnerable JavaScript libraries. One or more vulnerabilities were reported for this version of the library. Consult Attack details and Web References for more information about the affected library and the vulnerabilities that were reported.]]></Description>
        <DetailedInformation><![CDATA[]]></DetailedInformation>
        <Recommendation><![CDATA[Upgrade to the latest version.]]></Recommendation>
        <TechnicalDetails>
          <Request><![CDATA[GET /portal/portalhelp/en/website/help/ HTTP/1.1
Referer: http://testphp.vulnweb.com/
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
Accept-Encoding: gzip,deflate,br
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:87.0) Gecko/20100101 Firefox/87.0
Host: testphp.vulnweb.com
Connection: Keep-alive
]]></Request>
        </TechnicalDetails>
        <CWEList>
          <CWE id="937"><![CDATA[CWE-937]]></CWE>
        </CWEList>
        <CVEList>
        </CVEList>
        <CVSS>
          <Descriptor><![CDATA[AV:N/AC:L/Au:N/C:P/I:P/A:N]]></Descriptor>
          <Score><![CDATA[6.4]]></Score>
          <AV><![CDATA[Network_Accessible]]></AV>
          <AC><![CDATA[Low]]></AC>
          <Au><![CDATA[None]]></Au>
          <C><![CDATA[Partial]]></C>
          <I><![CDATA[Partial]]></I>
          <A><![CDATA[None]]></A>
          <E><![CDATA[]]></E>
          <RL><![CDATA[]]></RL>
          <RC><![CDATA[]]></RC>
        </CVSS>
        <CVSS3>
          <Descriptor><![CDATA[CVSS:3.0/AV:N/AC:L/PR:N/UI:N/S:U/C:L/I:L/A:N]]></Descriptor>
          <Score><![CDATA[6.5]]></Score>
          <TempScore><![CDATA[]]></TempScore>
          <EnvScore><![CDATA[]]></EnvScore>
          <AV><![CDATA[Network]]></AV>
          <AC><![CDATA[Low]]></AC>
          <PR><![CDATA[None]]></PR>
          <UI><![CDATA[None]]></UI>
          <S><![CDATA[Unchanged]]></S>
          <C><![CDATA[Low]]></C>
          <I><![CDATA[Low]]></I>
          <A><![CDATA[None]]></A>
          <E><![CDATA[]]></E>
          <RL><![CDATA[]]></RL>
          <RC><![CDATA[]]></RC>
        </CVSS3>
        <CVSS4>
          <Descriptor><![CDATA[CVSS:4.0/AV:N/AC:L/AT:N/PR:N/UI:N/VC:L/VI:L/VA:N/SC:N/SI:N/SA:N]]></Descriptor>
          <Score><![CDATA[6.9]]></Score>
          <AV><![CDATA[Network]]></AV>
          <AC><![CDATA[Low]]></AC>
          <PR><![CDATA[None]]></PR>
          <UI><![CDATA[None]]></UI>
          <VC><![CDATA[Low]]></VC>
          <VI><![CDATA[Low]]></VI>
          <VA><![CDATA[None]]></VA>
          <SC><![CDATA[None]]></SC>
          <SI><![CDATA[None]]></SI>
          <SA><![CDATA[None]]></SA>
        </CVSS4>
        <References>
        </References>
      </ReportItem>

Then, using the latest version of the dradis-acunetix plugin (currently https://github.com/dradis/dradis-acunetix/tree/release-4.10.0), I imported the XML on Dradis. Assuming you already have a Dradis project created, you need to:

  1. visit the endpoint https://<DRADIS_BASE_URL>/pro/projects/<PROJECT_ID>/upload
  2. select the XML file modified previously
  3. wait for it to upload

Once the upload is completed, among the issues you should have a new vulnerability named Vulnerable JavaScript libraries with one evidence.

Checking the evidence, you'll notice that the HTML lists are not nested correctly, but they have the same style and indentation, as you can see below.

image

Applying the modifications specified in this pull request, it is possible to solve this problem and correctly manage nested lists.

Follows a screenshot of the evidence Vulnerable JavaScript libraries, using the fix from this pull request.

image

[...] thanks for submitting this. Please review the CONTRIBUTING.md, and ensure you include:

  • a CHANGELOG entry
  • specs for your changes
  • testing steps in your pull request's description

Regarding the third point, I need more information as I'm not really familiar with "spec" and Ruby. Let me know if the file you want me to modify is https://raw.githubusercontent.com/DevNeol/dradis-acunetix/main/spec/dradis-acunetix_spec.rb, to add some tests for this functionality.

@MattBudz
Copy link
Contributor

@rbctee thanks for that, I have gone ahead and updated the PR description with the testing steps provided.

As for specs, you are correct, /spec/dradis-acunetix_spec.rb needs an update with a test for this functionality. This is required to ensure that future changes to this integration don't break this functionality. Let me know if you have any issues with this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@DevNeol DevNeol

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@DevNeol @MattBudz @rbctee