You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@santuario.apache.org by bu...@apache.org on 2008/05/09 02:00:34 UTC
DO NOT REPLY [Bug 44956] New: Concurrent creation of a XMLSignature
instance produces an ArrayIndexOutOfBoundsException
https://issues.apache.org/bugzilla/show_bug.cgi?id=44956
Summary: Concurrent creation of a XMLSignature instance produces
an ArrayIndexOutOfBoundsException
Product: Security
Version: unspecified
Platform: PC
OS/Version: All
Status: NEW
Severity: critical
Priority: P2
Component: Signature
AssignedTo: security-dev@xml.apache.org
ReportedBy: giedrius.noreikis@gmail.com
We are having problems with an org.apache.xml.security.signature.XMLSignature
instance creation in a multi-threaded environment: sometimes an
ArrayIndexOutOfBoundsException is thrown:
java.lang.ArrayIndexOutOfBoundsException: 38
at java.util.ArrayList.add(Unknown Source)
at org.apache.xml.security.keys.KeyInfo.<init>(Unknown Source)
at org.apache.xml.security.signature.XMLSignature.<init>(Unknown Source)
...
The XMLSignature constructor being used is:
public XMLSignature(Element element, String BaseURI)
Looking further at the sources I found out that:
1. The KeyInfo constructor being invoked by the XMLSignature constructor must
be public KeyInfo(Element element, String BaseURI) (XMLSignature:297).
2. The exact line producing the exception must be _storageResolvers.add(null)
(KeyInfo:123).
3. Upon the creation, the _storageResolvers variable of *each* KeyInfo instance
holds a reference to the *single* static nullList variable (KeyInfo:1067).
Thus, adding null to that list effectively modifies the single shared ArrayList
instance, while concurrent access and structural modifications of an ArrayList
instance are not allowed.
4. The entire _storageResolvers.add(null) statement seems to be useless and
probably could be simply removed.
5. IMHO, such a strange invention :) as that nullList should be evaluated and
probably removed as well.
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
DO NOT REPLY [Bug 44956] Concurrent creation of a XMLSignature
instance produces an ArrayIndexOutOfBoundsException
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=44956
--- Comment #2 from Giedrius Noreikis <gi...@gmail.com> 2008-05-13 16:23:53 PST ---
According to my tests, removing this line fixes the problem.
If more radical changes are undesirable due to a higher risk, to prevent such
bugs in future I would propose at least making that nullList final and
unmodifiable:
static final List nullList;
static {
List list = new ArrayList();
list.add(null);
nullList = Collections.unmodifiableList(list);
}
BTW, currently I'm working on a project using xmlsec library, and I have to fix
such issues anyway (just made own build).. So, in case you need help, I could
contribute more, by fixing bugs I've found, for example.
(In reply to comment #1)
> I'd like Raul to comment on this one, since I think this is part of his
> performance improvement changes. It seems that if we just removed line 123
> of KeyInfo.java, it would fix the problem:
> _storageResolvers.add(null);
> If there is a simple, low-risk fix then I am open to adding this to 1.4.2.
> (In reply to comment #0)
> > We are having problems with an org.apache.xml.security.signature.XMLSignature
> > instance creation in a multi-threaded environment: sometimes an
> > ArrayIndexOutOfBoundsException is thrown:
> >
> > java.lang.ArrayIndexOutOfBoundsException: 38
> > at java.util.ArrayList.add(Unknown Source)
> > at org.apache.xml.security.keys.KeyInfo.<init>(Unknown Source)
> > at org.apache.xml.security.signature.XMLSignature.<init>(Unknown Source)
> > ...
> >
> > The XMLSignature constructor being used is:
> > public XMLSignature(Element element, String BaseURI)
> >
> > Looking further at the sources I found out that:
> > 1. The KeyInfo constructor being invoked by the XMLSignature constructor must
> > be public KeyInfo(Element element, String BaseURI) (XMLSignature:297).
> > 2. The exact line producing the exception must be _storageResolvers.add(null)
> > (KeyInfo:123).
> > 3. Upon the creation, the _storageResolvers variable of *each* KeyInfo instance
> > holds a reference to the *single* static nullList variable (KeyInfo:1067).
> > Thus, adding null to that list effectively modifies the single shared ArrayList
> > instance, while concurrent access and structural modifications of an ArrayList
> > instance are not allowed.
> > 4. The entire _storageResolvers.add(null) statement seems to be useless and
> > probably could be simply removed.
> > 5. IMHO, such a strange invention :) as that nullList should be evaluated and
> > probably removed as well.
> >
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
DO NOT REPLY [Bug 44956] Concurrent creation of a XMLSignature
instance produces an ArrayIndexOutOfBoundsException
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=44956
Raul Benito <ra...@r-bg.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution| |FIXED
--- Comment #3 from Raul Benito <ra...@r-bg.com> 2008-05-14 14:11:29 PST ---
Good Analysis. I don't know why I forgot to remove add(null) in the contractor
that was the purpose of nullList, don't create and add a null to a list that
98% percent of the time will just have a null. And not to change the rest of
the code that depend on the null, as we don't have enough (any?) testcases that
check that behavior.
I have just add all the lines(including the unmodifiableList) even after
checking that the only add left checks before if the list is the null one. It
looks like a good way to detect the next error.
Thanks for detecting and for the report.
Regards
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
DO NOT REPLY [Bug 44956] Concurrent creation of a XMLSignature
instance produces an ArrayIndexOutOfBoundsException
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=44956
--- Comment #1 from sean.mullan@sun.com 2008-05-13 07:49:14 PST ---
I'd like Raul to comment on this one, since I think this is part of his
performance improvement changes. It seems that if we just removed line 123
of KeyInfo.java, it would fix the problem:
_storageResolvers.add(null);
If there is a simple, low-risk fix then I am open to adding this to 1.4.2.
(In reply to comment #0)
> We are having problems with an org.apache.xml.security.signature.XMLSignature
> instance creation in a multi-threaded environment: sometimes an
> ArrayIndexOutOfBoundsException is thrown:
>
> java.lang.ArrayIndexOutOfBoundsException: 38
> at java.util.ArrayList.add(Unknown Source)
> at org.apache.xml.security.keys.KeyInfo.<init>(Unknown Source)
> at org.apache.xml.security.signature.XMLSignature.<init>(Unknown Source)
> ...
>
> The XMLSignature constructor being used is:
> public XMLSignature(Element element, String BaseURI)
>
> Looking further at the sources I found out that:
> 1. The KeyInfo constructor being invoked by the XMLSignature constructor must
> be public KeyInfo(Element element, String BaseURI) (XMLSignature:297).
> 2. The exact line producing the exception must be _storageResolvers.add(null)
> (KeyInfo:123).
> 3. Upon the creation, the _storageResolvers variable of *each* KeyInfo instance
> holds a reference to the *single* static nullList variable (KeyInfo:1067).
> Thus, adding null to that list effectively modifies the single shared ArrayList
> instance, while concurrent access and structural modifications of an ArrayList
> instance are not allowed.
> 4. The entire _storageResolvers.add(null) statement seems to be useless and
> probably could be simply removed.
> 5. IMHO, such a strange invention :) as that nullList should be evaluated and
> probably removed as well.
>
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.