Disable DTDs for XML parsing

[ratulm]

No description provided.

[batfish-bot]

This change is 

[progwriter]

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @progwriter and @ratulm)

---

projects/batfish/src/main/java/org/batfish/representation/aws/VpnConnection.java, line 261 at r1 (raw file):

      DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
      // safe parser configuration -- disallows doctypes
      factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);

don't you (also?) need: setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);

[codecov]

Codecov Report

Merging #6094 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##             master    #6094   +/-   ##
=========================================
  Coverage     72.68%   72.69%           
- Complexity    34649    34650    +1     
=========================================
  Files          2815     2815           
  Lines        141416   141417    +1     
  Branches      16968    16968           
=========================================
+ Hits         102794   102798    +4     
+ Misses        30452    30450    -2     
+ Partials       8170     8169    -1     

Impacted Files	Coverage Δ	Complexity Δ
.../org/batfish/representation/aws/VpnConnection.java	72.34% <100.00%> (+0.11%)	32.00 <0.00> (ø)
...ain/java/org/batfish/symbolic/IngressLocation.java	65.78% <0.00%> (-2.64%)	15.00% <0.00%> (-1.00%)
...src/main/java/org/batfish/coordinator/WorkMgr.java	77.38% <0.00%> (-0.40%)	284.00% <0.00%> (-1.00%)
...java/org/batfish/dataplane/ibdp/VirtualRouter.java	88.00% <0.00%> (+0.21%)	264.00% <0.00%> (+1.00%)
...ain/java/org/batfish/storage/FileBasedStorage.java	86.85% <0.00%> (+0.26%)	254.00% <0.00%> (ø%)
...c/main/java/org/batfish/dataplane/rib/RibTree.java	93.75% <0.00%> (+4.16%)	28.00% <0.00%> (+1.00%)
.../org/batfish/dataplane/rib/RouteAdvertisement.java	89.13% <0.00%> (+4.34%)	17.00% <0.00%> (+1.00%)
...rg/batfish/identifiers/StorageBasedIdResolver.java	93.33% <0.00%> (+4.44%)	27.00% <0.00%> (ø%)

[ratulm]

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @progwriter)

---

projects/batfish/src/main/java/org/batfish/representation/aws/VpnConnection.java, line 261 at r1 (raw file):

Previously, progwriter (Victor Heorhiadi) wrote…

don't you (also?) need: setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);

Do I?

I was getting info from https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html.

I couldn't really tell if external-dtd needs to be disabled when i've disabled all dtds. I don't really know this stuff. do you?

[progwriter]

Reviewed 1 of 1 files at r1.
Reviewable status: complete! all files reviewed, all discussions resolved

---

projects/batfish/src/main/java/org/batfish/representation/aws/VpnConnection.java, line 261 at r1 (raw file):

Previously, ratulm wrote…

Do I?

I was getting info from https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html.

I couldn't really tell if external-dtd needs to be disabled when i've disabled all dtds. I don't really know this stuff. do you?

still figuring it out. was looking here: https://xerces.apache.org/xerces2-j/features.html#disallow-doctype-decl
and here https://xerces.apache.org/xerces2-j/features.html#nonvalidating.load-external-dtd

Looks like disallow-doctype-decl does take precedence, but will crash if xml contains a DOCTYPE declaration. assuming aws won't send us xml like that.