santuario-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Sean Mullan <sean.mul...@oracle.com>
Subject Re: XML Security Java 1.5.0-RC1 available
Date Tue, 20 Dec 2011 19:11:48 GMT
On 12/20/2011 11:21 AM, Cantor, Scott wrote:
> On 12/20/11 11:12 AM, "Sean Mullan"<sean.mullan@oracle.com>  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

Mime
View raw message