8012288: XML DSig API allows wrong tag names and extra elements in SignedInfo

Reviewed-by: xuelei

src/share/classes/org/jcp/xml/dsig/internal/dom/DOMKeyValue.java

@@ -218,9 +218,11 @@ public abstract class DOMKeyValue extends DOMStructure implements KeyValue {
                         ("unable to create RSA KeyFactory: " + e.getMessage());
                 }
             }
-            Element modulusElem = DOMUtils.getFirstChildElement(kvtElem);
+            Element modulusElem = DOMUtils.getFirstChildElement(kvtElem,
+                                                                "Modulus");
             modulus = new DOMCryptoBinary(modulusElem.getFirstChild());
-            Element exponentElem = DOMUtils.getNextSiblingElement(modulusElem);
+            Element exponentElem = DOMUtils.getNextSiblingElement(modulusElem,
+                                                                  "Exponent");
             exponent = new DOMCryptoBinary(exponentElem.getFirstChild());
             RSAPublicKeySpec spec = new RSAPublicKeySpec(modulus.getBigNum(),
                                                          exponent.getBigNum());
@@ -289,13 +291,13 @@ public abstract class DOMKeyValue extends DOMStructure implements KeyValue {
             // check for P and Q
             if (curElem.getLocalName().equals("P")) {
                 p = new DOMCryptoBinary(curElem.getFirstChild());
-                curElem = DOMUtils.getNextSiblingElement(curElem);
+                curElem = DOMUtils.getNextSiblingElement(curElem, "Q");
                 q = new DOMCryptoBinary(curElem.getFirstChild());
                 curElem = DOMUtils.getNextSiblingElement(curElem);
             }
             if (curElem.getLocalName().equals("G")) {
                 g = new DOMCryptoBinary(curElem.getFirstChild());
-                curElem = DOMUtils.getNextSiblingElement(curElem);
+                curElem = DOMUtils.getNextSiblingElement(curElem, "Y");
             }
             y = new DOMCryptoBinary(curElem.getFirstChild());
             curElem = DOMUtils.getNextSiblingElement(curElem);
@@ -460,7 +462,7 @@ public abstract class DOMKeyValue extends DOMStructure implements KeyValue {
             } else {
                 throw new MarshalException("Invalid ECKeyValue");
             }
-            curElem = DOMUtils.getNextSiblingElement(curElem);
+            curElem = DOMUtils.getNextSiblingElement(curElem, "PublicKey");
             ECPoint ecPoint = null;
             try {
                 Object[] args = new Object[] { Base64.decode(curElem),

src/share/classes/org/jcp/xml/dsig/internal/dom/DOMManifest.java

@@ -101,20 +101,24 @@ public final class DOMManifest extends DOMStructure implements Manifest {
 
         boolean secVal = Utils.secureValidation(context);
 
-        Element refElem = DOMUtils.getFirstChildElement(manElem);
+        Element refElem = DOMUtils.getFirstChildElement(manElem, "Reference");
         List<Reference> refs = new ArrayList<Reference>();
+        refs.add(new DOMReference(refElem, context, provider));
 
-        int refCount = 0;
+        refElem = DOMUtils.getNextSiblingElement(refElem);
         while (refElem != null) {
+            String localName = refElem.getLocalName();
+            if (!localName.equals("Reference")) {
+                throw new MarshalException("Invalid element name: " +
+                                           localName + ", expected Reference");
+            }
             refs.add(new DOMReference(refElem, context, provider));
-            refElem = DOMUtils.getNextSiblingElement(refElem);
-
-            refCount++;
-            if (secVal && (refCount > DOMSignedInfo.MAXIMUM_REFERENCE_COUNT)) {
+            if (secVal && (refs.size() > DOMSignedInfo.MAXIMUM_REFERENCE_COUNT)) {
                 String error = "A maxiumum of " + DOMSignedInfo.MAXIMUM_REFERENCE_COUNT + " "
                     + "references per Manifest are allowed with secure validation";
                 throw new MarshalException(error);
             }
+            refElem = DOMUtils.getNextSiblingElement(refElem);
         }
         this.references = Collections.unmodifiableList(refs);
     }

src/share/classes/org/jcp/xml/dsig/internal/dom/DOMReference.java

@@ -204,23 +204,33 @@ public final class DOMReference extends DOMStructure
         Element nextSibling = DOMUtils.getFirstChildElement(refElem);
         List<Transform> transforms = new ArrayList<Transform>(5);
         if (nextSibling.getLocalName().equals("Transforms")) {
-            Element transformElem = DOMUtils.getFirstChildElement(nextSibling);
-
-            int transformCount = 0;
+            Element transformElem = DOMUtils.getFirstChildElement(nextSibling,
+                                                                  "Transform");
+            transforms.add(new DOMTransform(transformElem, context, provider));
+            transformElem = DOMUtils.getNextSiblingElement(transformElem);
             while (transformElem != null) {
+                String localName = transformElem.getLocalName();
+                if (!localName.equals("Transform")) {
+                    throw new MarshalException(
+                        "Invalid element name: " + localName +
+                        ", expected Transform");
+                }
                 transforms.add
                     (new DOMTransform(transformElem, context, provider));
-                transformElem = DOMUtils.getNextSiblingElement(transformElem);
-
-                transformCount++;
-                if (secVal && (transformCount > MAXIMUM_TRANSFORM_COUNT)) {
+                if (secVal && (transforms.size() > MAXIMUM_TRANSFORM_COUNT)) {
                     String error = "A maxiumum of " + MAXIMUM_TRANSFORM_COUNT + " "
                         + "transforms per Reference are allowed with secure validation";
                     throw new MarshalException(error);
                 }
+                transformElem = DOMUtils.getNextSiblingElement(transformElem);
             }
             nextSibling = DOMUtils.getNextSiblingElement(nextSibling);
         }
+        if (!nextSibling.getLocalName().equals("DigestMethod")) {
+            throw new MarshalException("Invalid element name: " +
+                                       nextSibling.getLocalName() +
+                                       ", expected DigestMethod");
+        }
 
         // unmarshal DigestMethod
         Element dmElem = nextSibling;
@@ -234,13 +244,19 @@ public final class DOMReference extends DOMStructure
         }
 
         // unmarshal DigestValue
+        Element dvElem = DOMUtils.getNextSiblingElement(dmElem, "DigestValue");
         try {
-            Element dvElem = DOMUtils.getNextSiblingElement(dmElem);
             this.digestValue = Base64.decode(dvElem);
         } catch (Base64DecodingException bde) {
             throw new MarshalException(bde);
         }
 
+        // check for extra elements
+        if (DOMUtils.getNextSiblingElement(dvElem) != null) {
+            throw new MarshalException(
+                "Unexpected element after DigestValue element");
+        }
+
         // unmarshal attributes
         this.uri = DOMUtils.getAttributeValue(refElem, "URI");
 

src/share/classes/org/jcp/xml/dsig/internal/dom/DOMRetrievalMethod.java

@@ -136,21 +136,30 @@ public final class DOMRetrievalMethod extends DOMStructure
         List<Transform> transforms = new ArrayList<Transform>();
         Element transformsElem = DOMUtils.getFirstChildElement(rmElem);
 
-        int transformCount = 0;
         if (transformsElem != null) {
+            String localName = transformsElem.getLocalName();
+            if (!localName.equals("Transforms")) {
+                throw new MarshalException("Invalid element name: " +
+                                           localName + ", expected Transforms");
+            }
             Element transformElem =
-                DOMUtils.getFirstChildElement(transformsElem);
+                DOMUtils.getFirstChildElement(transformsElem, "Transform");
+            transforms.add(new DOMTransform(transformElem, context, provider));
+            transformElem = DOMUtils.getNextSiblingElement(transformElem);
             while (transformElem != null) {
+                String name = transformElem.getLocalName();
+                if (!name.equals("Transform")) {
+                    throw new MarshalException("Invalid element name: " +
+                                               name + ", expected Transform");
+                }
                 transforms.add
                     (new DOMTransform(transformElem, context, provider));
-                transformElem = DOMUtils.getNextSiblingElement(transformElem);
-
-                transformCount++;
-                if (secVal && (transformCount > DOMReference.MAXIMUM_TRANSFORM_COUNT)) {
+                if (secVal && (transforms.size() > DOMReference.MAXIMUM_TRANSFORM_COUNT)) {
                     String error = "A maxiumum of " + DOMReference.MAXIMUM_TRANSFORM_COUNT + " "
                         + "transforms per Reference are allowed with secure validation";
                     throw new MarshalException(error);
                 }
+                transformElem = DOMUtils.getNextSiblingElement(transformElem);
             }
         }
         if (transforms.isEmpty()) {

src/share/classes/org/jcp/xml/dsig/internal/dom/DOMSignatureProperties.java

@@ -109,6 +109,11 @@ public final class DOMSignatureProperties extends DOMStructure
         for (int i = 0; i < length; i++) {
             Node child = nodes.item(i);
             if (child.getNodeType() == Node.ELEMENT_NODE) {
+                String name = child.getLocalName();
+                if (!name.equals("SignatureProperty")) {
+                    throw new MarshalException("Invalid element name: " + name +
+                                               ", expected SignatureProperty");
+                }
                 properties.add(new DOMSignatureProperty((Element)child,
                                                         context));
             }

src/share/classes/org/jcp/xml/dsig/internal/dom/DOMSignedInfo.java

@@ -150,11 +150,14 @@ public final class DOMSignedInfo extends DOMStructure implements SignedInfo {
         id = DOMUtils.getAttributeValue(siElem, "Id");
 
         // unmarshal CanonicalizationMethod
-        Element cmElem = DOMUtils.getFirstChildElement(siElem);
-        canonicalizationMethod = new DOMCanonicalizationMethod(cmElem, context, provider);
+        Element cmElem = DOMUtils.getFirstChildElement(siElem,
+                                                       "CanonicalizationMethod");
+        canonicalizationMethod = new DOMCanonicalizationMethod(cmElem, context,
+                                                               provider);
 
         // unmarshal SignatureMethod
-        Element smElem = DOMUtils.getNextSiblingElement(cmElem);
+        Element smElem = DOMUtils.getNextSiblingElement(cmElem,
+                                                        "SignatureMethod");
         signatureMethod = DOMSignatureMethod.unmarshal(smElem);
 
         boolean secVal = Utils.secureValidation(context);
@@ -169,19 +172,24 @@ public final class DOMSignedInfo extends DOMStructure implements SignedInfo {
 
         // unmarshal References
         ArrayList<Reference> refList = new ArrayList<Reference>(5);
-        Element refElem = DOMUtils.getNextSiblingElement(smElem);
+        Element refElem = DOMUtils.getNextSiblingElement(smElem, "Reference");
+        refList.add(new DOMReference(refElem, context, provider));
 
-        int refCount = 0;
+        refElem = DOMUtils.getNextSiblingElement(refElem);
         while (refElem != null) {
+            String name = refElem.getLocalName();
+            if (!name.equals("Reference")) {
+                throw new MarshalException("Invalid element name: " +
+                                           name + ", expected Reference");
+            }
             refList.add(new DOMReference(refElem, context, provider));
-            refElem = DOMUtils.getNextSiblingElement(refElem);
 
-            refCount++;
-            if (secVal && (refCount > MAXIMUM_REFERENCE_COUNT)) {
+            if (secVal && (refList.size() > MAXIMUM_REFERENCE_COUNT)) {
                 String error = "A maxiumum of " + MAXIMUM_REFERENCE_COUNT + " "
                     + "references per Manifest are allowed with secure validation";
                 throw new MarshalException(error);
             }
+            refElem = DOMUtils.getNextSiblingElement(refElem);
         }
         references = Collections.unmodifiableList(refList);
     }

src/share/classes/org/jcp/xml/dsig/internal/dom/DOMUtils.java

@@ -131,6 +131,36 @@ public class DOMUtils {
         return (Element)child;
     }
 
+    /**
+     * Returns the first child element of the specified node and checks that
+     * the local name is equal to {@code localName}.
+     *
+     * @param node the node
+     * @return the first child element of the specified node
+     * @throws NullPointerException if {@code node == null}
+     * @throws MarshalException if no such element or the local name is not
+     *    equal to {@code localName}
+     */
+    public static Element getFirstChildElement(Node node, String localName)
+        throws MarshalException
+    {
+        return verifyElement(getFirstChildElement(node), localName);
+    }
+
+    private static Element verifyElement(Element elem, String localName)
+        throws MarshalException
+    {
+        if (elem == null) {
+            throw new MarshalException("Missing " + localName + " element");
+        }
+        String name = elem.getLocalName();
+        if (!name.equals(localName)) {
+            throw new MarshalException("Invalid element name: " +
+                                       name + ", expected " + localName);
+        }
+        return elem;
+    }
+
     /**
      * Returns the last child element of the specified node, or null if there
      * is no such element.
@@ -165,6 +195,22 @@ public class DOMUtils {
         return (Element)sibling;
     }
 
+    /**
+     * Returns the next sibling element of the specified node and checks that
+     * the local name is equal to {@code localName}.
+     *
+     * @param node the node
+     * @return the next sibling element of the specified node
+     * @throws NullPointerException if {@code node == null}
+     * @throws MarshalException if no such element or the local name is not
+     *    equal to {@code localName}
+     */
+    public static Element getNextSiblingElement(Node node, String localName)
+        throws MarshalException
+    {
+        return verifyElement(getNextSiblingElement(node), localName);
+    }
+
     /**
      * Returns the attribute value for the attribute with the specified name.
      * Returns null if there is no such attribute, or

src/share/classes/org/jcp/xml/dsig/internal/dom/DOMX509IssuerSerial.java

@@ -80,9 +80,11 @@ public final class DOMX509IssuerSerial extends DOMStructure
      *
      * @param isElem an X509IssuerSerial element
      */
-    public DOMX509IssuerSerial(Element isElem) {
-        Element iNElem = DOMUtils.getFirstChildElement(isElem);
-        Element sNElem = DOMUtils.getNextSiblingElement(iNElem);
+    public DOMX509IssuerSerial(Element isElem) throws MarshalException {
+        Element iNElem = DOMUtils.getFirstChildElement(isElem,
+                                                       "X509IssuerName");
+        Element sNElem = DOMUtils.getNextSiblingElement(iNElem,
+                                                        "X509SerialNumber");
         issuerName = iNElem.getFirstChild().getNodeValue();
         serialNumber = new BigInteger(sNElem.getFirstChild().getNodeValue());
     }

src/share/classes/org/jcp/xml/dsig/internal/dom/DOMXMLSignature.java

@@ -141,11 +141,13 @@ public final class DOMXMLSignature extends DOMStructure
         id = DOMUtils.getAttributeValue(localSigElem, "Id");
 
         // unmarshal SignedInfo
-        Element siElem = DOMUtils.getFirstChildElement(localSigElem);
+        Element siElem = DOMUtils.getFirstChildElement(localSigElem,
+                                                       "SignedInfo");
         si = new DOMSignedInfo(siElem, context, provider);
 
         // unmarshal SignatureValue
-        Element sigValElem = DOMUtils.getNextSiblingElement(siElem);
+        Element sigValElem = DOMUtils.getNextSiblingElement(siElem,
+                                                            "SignatureValue");
         sv = new DOMSignatureValue(sigValElem, context);
 
         // unmarshal KeyInfo, if specified
@@ -161,6 +163,11 @@ public final class DOMXMLSignature extends DOMStructure
         } else {
             List<XMLObject> tempObjects = new ArrayList<XMLObject>();
             while (nextSibling != null) {
+                String name = nextSibling.getLocalName();
+                if (!name.equals("Object")) {
+                    throw new MarshalException("Invalid element name: " + name +
+                                               ", expected KeyInfo or Object");
+                }
                 tempObjects.add(new DOMXMLObject(nextSibling,
                                                  context, provider));
                 nextSibling = DOMUtils.getNextSiblingElement(nextSibling);