Additional nodes can be added to XML without breaking signature
Basics
Logistics
Basics
Logistics
Description
An outside tester demonstrated that a similar attack to the DTD issue allows comments to be inserted without breaking a signature if the c14n method excludes them. This similarly corrupts the text content surfaced by the library.
The "simple" fix for this would appear to be to ignore comments with the parser, which happens to be the default behavior of the Java library. The impact would likely be twofold:
serialization of metadata for backups would strip comments
metadata with comments signed using c14n methods that include them would not verify
We presume the latter has not been a practice we see with the IdP, so presumably wouldn't be common with the SP.
Environment
None
Activity
Scott Cantor
March 9, 2018 at 4:50 PM
The advisory has been updated with the link to the commit.
Unidentified Legacy Account
March 9, 2018 at 7:09 AM
(edited)
Does this link provides the correct diff from the branch
Can you provide me the link of the correct patch from the branch?
I am not able to find it
Thanks in advance
Scott Cantor
March 5, 2018 at 2:37 PM
You copied the wrong patch, that's from master, not the branch. And you would seemingly either have failed to patch the parser pool source file, or you are on such an old version of Xerces that you are dead anyway. Anybody using xmltooling code that old is screwed regardless.
The behaviour now we are seeing is inserting a XML Comment is throwing exception "Unmarshaller found unsupported node type" . Is the behaviour correct ?
I was expecting the changes to basically ignore the comment and combine the previous text and subsequent text together.
An outside tester demonstrated that a similar attack to the DTD issue allows comments to be inserted without breaking a signature if the c14n method excludes them. This similarly corrupts the text content surfaced by the library.
The "simple" fix for this would appear to be to ignore comments with the parser, which happens to be the default behavior of the Java library. The impact would likely be twofold:
serialization of metadata for backups would strip comments
metadata with comments signed using c14n methods that include them would not verify
We presume the latter has not been a practice we see with the IdP, so presumably wouldn't be common with the SP.