8278186: org.jcp.xml.dsig.internal.dom.Utils.parseIdFromSameDocumentURI throws StringIndexOutOfBoundsException when calling substring method

Reviewed-by: mullan

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

@@ -21,7 +21,7 @@
  * under the License.
  */
 /*
- * Copyright (c) 2005, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2005, 2021, Oracle and/or its affiliates. All rights reserved.
  */
 package org.jcp.xml.dsig.internal.dom;
 
@@ -101,8 +101,10 @@ public final class DOMURIDereferencer implements URIDereferencer {
             if (id.startsWith("xpointer(id(")) {
                 int i1 = id.indexOf('\'');
                 int i2 = id.indexOf('\'', i1+1);
+                if (i1 >= 0 && i2 >= 0) {
                     id = id.substring(i1 + 1, i2);
                 }
+            }
 
             // check if element is registered by Id
             Node referencedElem = uriAttr.getOwnerDocument().getElementById(id);

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

@@ -21,7 +21,7 @@
  * under the License.
  */
 /*
- * Copyright (c) 2005, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2005, 2021, Oracle and/or its affiliates. All rights reserved.
  */
 package org.jcp.xml.dsig.internal.dom;
 
@@ -94,8 +94,10 @@ public final class Utils {
         if (id.startsWith("xpointer(id(")) {
             int i1 = id.indexOf('\'');
             int i2 = id.indexOf('\'', i1+1);
+            if (i1 >= 0 && i2 >= 0) {
                 id = id.substring(i1 + 1, i2);
             }
+        }
         return id;
     }
 

test/jdk/javax/xml/crypto/dsig/BadXPointer.java

@@ -0,0 +1,60 @@
+/*
+ * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+import jdk.test.lib.Asserts;
+import jdk.test.lib.Utils;
+import jdk.test.lib.security.XMLUtils;
+
+import javax.xml.crypto.URIReferenceException;
+import javax.xml.crypto.dsig.XMLSignatureException;
+import java.security.KeyPair;
+import java.security.KeyPairGenerator;
+import java.security.spec.ECGenParameterSpec;
+
+/**
+ * @test
+ * @bug 8278186
+ * @summary reject malformed xpointer(id('a')) gracefully
+ * @library /test/lib
+ * @modules java.xml.crypto
+ */
+public class BadXPointer {
+
+    public static void main(String[] args) throws Exception {
+
+        KeyPairGenerator kpg = KeyPairGenerator.getInstance("EC");
+        kpg.initialize(new ECGenParameterSpec("secp256r1"));
+        KeyPair kp = kpg.generateKeyPair();
+
+        var signer = XMLUtils.signer(kp.getPrivate(), kp.getPublic());
+        var doc = XMLUtils.string2doc("<root/>");
+
+        // No enclosing ' for id
+        Utils.runAndCheckException(
+                () -> signer.signEnveloping(doc, "a", "#xpointer(id('a))"),
+                ex -> Asserts.assertTrue(ex instanceof XMLSignatureException
+                        && ex.getCause() instanceof URIReferenceException
+                        && ex.getMessage().contains("Could not find a resolver"),
+                    ex.toString()));
+    }
+}

test/lib/jdk/test/lib/security/XMLUtils.java

@@ -92,9 +92,11 @@ public class XMLUtils {
         Asserts.assertTrue(v2.validate(s3.sign(d))); // can read KeyInfo
         Asserts.assertTrue(v2.secureValidation(false).validate(s3.sign(p.toUri()))); // can read KeyInfo
         Asserts.assertTrue(v2.secureValidation(false).baseURI(b).validate(
-                s3.sign(p.getParent().toUri(), p.getFileName().toUri()))); // can read KeyInfo
+                s3.sign(p.toAbsolutePath().getParent().toUri(), p.getFileName().toUri()))); // can read KeyInfo
         Asserts.assertTrue(v1.validate(s1.sign("text"))); // plain text
         Asserts.assertTrue(v1.validate(s1.sign("binary".getBytes()))); // raw data
+        Asserts.assertTrue(v1.validate(s1.signEnveloping(d, "x", "#x")));
+        Asserts.assertTrue(v1.validate(s1.signEnveloping(d, "x", "#xpointer(id('x'))")));
     }
 
     //////////// CONVERT ////////////
@@ -347,14 +349,14 @@ public class XMLUtils {
         }
 
         // Signs a document in enveloping mode
-        public Document signEnveloping(Document document) throws Exception {
+        public Document signEnveloping(Document document, String id, String ref) throws Exception {
             Document newDocument = DocumentBuilderFactory.newInstance()
                     .newDocumentBuilder().newDocument();
             FAC.newXMLSignature(
-                    buildSignedInfo(FAC.newReference("#object", dm)),
+                    buildSignedInfo(FAC.newReference(ref, dm)),
                     buildKeyInfo(),
                     List.of(FAC.newXMLObject(List.of(new DOMStructure(document.getDocumentElement())),
-                            "object", null, null)),
+                            id, null, null)),
                     null,
                     null)
                     .sign(new DOMSignContext(privateKey, newDocument));
@@ -474,7 +476,7 @@ public class XMLUtils {
         // If key is not null, any key from the signature will be ignored
         public boolean validate(Document document, PublicKey key)
                 throws Exception {
-            NodeList nodeList = document.getElementsByTagName("Signature");
+            NodeList nodeList = document.getElementsByTagNameNS(XMLSignature.XMLNS, "Signature");
             if (nodeList.getLength() == 1) {
                 Node signatureNode = nodeList.item(0);
                 if (signatureNode != null) {