On 12/20/2011 11:21 AM, Cantor, Scott wrote:
On 12/20/11 11:12 AM, "Sean Mullan"<[email protected]> wrote:
The code does still call DOM Document.getElementById, but how does that
make it possible to do an attack?
If you mark "known" ID attributes as IDs by calling setIdAtribute, Xerces
will allow duplicates.
I did a grep of the source tree, and our code never calls
Element.setIdAttribute.
If you insist that the application do a tree walk
to prevent duplicates, then you're telling us that every app should
implement the thing you claim is too slow for Santuario to implement.
Usually an app won't have to do that - it is aware of the schema, and
where the element IDs should be in the document - Colm can comment some
more on how the WSS library does this. Our library won't find anything
that isn't registered, so if you stick something way down in the guts of
the document, it simply won't find it. (It used to, but not as of 1.5).
But I can see the duplicate ID issues you mention, if the app uses
Element.setIdAttribute to register the ID attributes.
As I understand the wrapping attacks, it happens after the signature is
validated, when the application actually acts on the element content
that is mapped to that ID. Then, it needs to find that element, and if
there are duplicate IDs and it gets the wrong one, then oops. As Colm
mentioned, we do have a mechanism to return the Elements that were
actually validated.
But I guess I see an issue in that it is hard for the app to do all
these extra checks to prevent wrapping attacks. It sounds like what we
need is an additional optional "sanity" check on the entire document
looking for duplicate IDs.
Additionally, parsers have bugs enforcing uniqueness on IDs during schema
validation. Xerces-C for example has a nasty one involving wildcards and
lax schemas. All because they allow something that shouldn't ever be legal
in the first place, duplicates in the Document's ID map. Fix that, in one
place, and all of this goes away.
Thus my point. The Xerces team is wrong. Somebody needs to explain that to
them, somebody they'll listen to.
If they won't listen to you, I'm sure they won't listen to me ;)
The trusted validation code should be
creating the Document and registering the IDs. If you are letting
untrusted code create the Document for you and register arbitrary IDs,
then that is a bug.
If you claim schema validation is "one way", then the parser has to be
trusted. Alternatively, the application can't just register the IDs, but
must also track and search for duplicates, or it can't rely on
setIdAttributeNS to register them.
That's why I'm asking about the algorithm. You're basically duplicating
the DOM3 ID API in Santuario. So my advice is to stop relying on the
original. Since I don't think that's really a good thing either, I
continue to argue that the fix belongs in the parser.
Hmm, I suppose we could stop calling Document.getElementById if the
document was not validated against a schema. Let me think about that
some more.
--Sean
