You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jena.apache.org by rv...@apache.org on 2018/03/05 14:54:51 UTC

[1/3] jena git commit: Fix delimiter parsing logic (JENA-1497)

Repository: jena
Updated Branches:
  refs/heads/master b75fccefa -> 2e732b485


Fix delimiter parsing logic (JENA-1497)

Logical flaws in using continue vs break inside inner loops where
causing the wrong delimiter positions to be detected and leading to
false positives being reported for potential injection attacks.  Fixing
the logic allows the user test case to pass.


Project: http://git-wip-us.apache.org/repos/asf/jena/repo
Commit: http://git-wip-us.apache.org/repos/asf/jena/commit/411c1031
Tree: http://git-wip-us.apache.org/repos/asf/jena/tree/411c1031
Diff: http://git-wip-us.apache.org/repos/asf/jena/diff/411c1031

Branch: refs/heads/master
Commit: 411c1031a0f51885f6966914c58202654614be13
Parents: b75fcce
Author: Rob Vesse <rv...@apache.org>
Authored: Mon Mar 5 10:25:24 2018 +0000
Committer: Rob Vesse <rv...@apache.org>
Committed: Mon Mar 5 10:25:24 2018 +0000

----------------------------------------------------------------------
 .../jena/query/ParameterizedSparqlString.java   | 30 ++++++++----
 .../query/TestParameterizedSparqlString.java    | 49 ++++++++++++++++++++
 2 files changed, 70 insertions(+), 9 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/jena/blob/411c1031/jena-arq/src/main/java/org/apache/jena/query/ParameterizedSparqlString.java
----------------------------------------------------------------------
diff --git a/jena-arq/src/main/java/org/apache/jena/query/ParameterizedSparqlString.java b/jena-arq/src/main/java/org/apache/jena/query/ParameterizedSparqlString.java
index f421d37..0e3e150 100644
--- a/jena-arq/src/main/java/org/apache/jena/query/ParameterizedSparqlString.java
+++ b/jena-arq/src/main/java/org/apache/jena/query/ParameterizedSparqlString.java
@@ -1558,9 +1558,9 @@ public class ParameterizedSparqlString implements PrefixMapping {
                             if (cs[j] == '"' && cs[j + 1] == '"' && cs[j + 2] == '"') {
                                 this.addStop(i, j + 2);
                                 i = j + 2;
+                                break;
                             }
                         }
-                        // Was unterminated
                     } else {
                         // Normal literal, scan till we see a " which is not
                         // preceded by a \
@@ -1569,10 +1569,9 @@ public class ParameterizedSparqlString implements PrefixMapping {
                             if (cs[j] == '"' && cs[j - 1] != '\\') {
                                 this.addStop(i, j);
                                 i = j;
-                                continue;
+                                break;
                             }
                         }
-                        // Was unterminated
                     }
                     break;
                 case '<':
@@ -1582,10 +1581,9 @@ public class ParameterizedSparqlString implements PrefixMapping {
                         if (cs[j] == '>' && cs[j - 1] != '\\') {
                             this.addStop(i, j);
                             i = j;
-                            continue;
+                            break;
                         }
                     }
-                    // Was unterminated
                     break;
                 case '\'':
                     // Start of alternative literal form
@@ -1597,9 +1595,9 @@ public class ParameterizedSparqlString implements PrefixMapping {
                             if (cs[j] == '\'' && cs[j + 1] == '\'' && cs[j + 2] == '\'') {
                                 this.addStop(i, j + 2);
                                 i = j + 2;
+                                break;
                             }
                         }
-                        // Was unterminated
                     } else {
                         // Normal literal, scan till we see a ' which is not
                         // preceded by a \
@@ -1608,10 +1606,9 @@ public class ParameterizedSparqlString implements PrefixMapping {
                             if (cs[j] == '\'' && cs[j - 1] != '\\') {
                                 this.addStop(i, j);
                                 i = j;
-                                continue;
+                                break;
                             }
                         }
-                        // Was unterminated
                     }
                     break;
                 case '#':
@@ -1622,7 +1619,7 @@ public class ParameterizedSparqlString implements PrefixMapping {
                         if (cs[j] == '\n' || cs[j] == '\r') {
                             this.addStop(i, j);
                             i = j;
-                            continue;
+                            break;
                         }
                     }
                     this.addStop(i, cs.length - 1);
@@ -1720,6 +1717,21 @@ public class ParameterizedSparqlString implements PrefixMapping {
                 return false;
             }
         }
+        
+        @Override
+        public String toString() {
+            StringBuilder builder = new StringBuilder();
+            for (Pair<Integer, String> pair : this.starts) {
+                builder.append("Delim ");
+                builder.append(pair.getRight());
+                builder.append(" - Start ");
+                builder.append(pair.getLeft());
+                builder.append(" - End ");
+                builder.append(this.stops.get(pair.getLeft()));
+                builder.append('\n');
+            }
+            return builder.toString();
+        }
 
     }
 }

http://git-wip-us.apache.org/repos/asf/jena/blob/411c1031/jena-arq/src/test/java/org/apache/jena/query/TestParameterizedSparqlString.java
----------------------------------------------------------------------
diff --git a/jena-arq/src/test/java/org/apache/jena/query/TestParameterizedSparqlString.java b/jena-arq/src/test/java/org/apache/jena/query/TestParameterizedSparqlString.java
index 7335f75..2f928e7 100644
--- a/jena-arq/src/test/java/org/apache/jena/query/TestParameterizedSparqlString.java
+++ b/jena-arq/src/test/java/org/apache/jena/query/TestParameterizedSparqlString.java
@@ -19,6 +19,7 @@
 package org.apache.jena.query;
 
 import java.util.Calendar ;
+import java.util.HashMap;
 import java.util.Iterator ;
 import java.util.TimeZone ;
 
@@ -1591,6 +1592,32 @@ public class TestParameterizedSparqlString {
         UpdateRequest updates = pss.asUpdate();
         Assert.fail("Attempt to do SPARQL injection should result in an exception");
     }
+    
+
+    
+    @Test
+    public void test_param_string_injection_16() {
+        String prefixes="PREFIX : <http://purl.bdrc.io/ontology/core/>\n" +
+                " PREFIX skos: <http://www.w3.org/2004/02/skos/core#>\n" +
+                " PREFIX text: <http://jena.apache.org/text#>\n" ;
+        HashMap<String,String> map=new HashMap<>();
+        map.put("L_name", "\"rgyud bla ma\"");
+        map.put("LG_name", "bo-x-ewts");
+        String test2=prefixes+ "select ?comment (GROUP_CONCAT(DISTINCT ?comment_type;  SEPARATOR=\" <>" +
+                "\") AS ?comment_types)  ?root_name\n" +
+                "where {\n" +
+                "    (?root ?score ?root_name) text:query ?L_name .\n" +
+                "    ?comment :workIsAbout ?root;\n" +
+                "             :workGenre ?g .\n" +
+                "    ?g skos:prefLabel ?comment_type .\n" +
+                "    FILTER (contains(?comment_type, \"commentary\" ))\n" +
+                "}\n" +
+                "group by ?comment ?root_name";
+        ParameterizedSparqlString queryStr2 = new ParameterizedSparqlString(test2);
+        queryStr2.setLiteral("L_name", map.get("L_name"),map.get("LG_name"));
+        System.out.println(queryStr2.toString());
+        Query q2=queryStr2.asQuery();
+    }
 
     @Test
     public void test_param_string_non_injection_01() {
@@ -1603,6 +1630,28 @@ public class TestParameterizedSparqlString {
 
         pss.toString();
     }
+    
+    @Test
+    public void test_param_string_non_injection_02() {
+        String prefixes="PREFIX : <http://purl.bdrc.io/ontology/core/>\n" +
+                " PREFIX skos: <http://www.w3.org/2004/02/skos/core#>\n" +
+                " PREFIX text: <http://jena.apache.org/text#>" ;
+        HashMap<String,String> map=new HashMap<>();
+        map.put("L_name", "\"rgyud bla ma\"");
+        map.put("LG_name", "bo-x-ewts");        
+        String test1=prefixes+ "select ?comment (GROUP_CONCAT(DISTINCT ?comment_type;  SEPARATOR=\" <>" +
+                "\") AS ?comment_types)  ?root_name\n" +
+                "where {\n" +
+                "    (?root ?score ?root_name) text:query ?L_name .\n" +
+                "    ?comment :workIsAbout ?root;\n" +
+                "             :workGenre ?g .\n" +
+                "    ?g skos:prefLabel ?comment_type .\n" +
+                "}\n"+
+                "group by ?comment ?root_name";              
+        ParameterizedSparqlString queryStr = new ParameterizedSparqlString(test1);
+        queryStr.setLiteral("L_name", map.get("L_name"),map.get("LG_name"));        
+        queryStr.asQuery();
+    }
 
     @Test(expected = ARQException.class)
     public void test_param_string_positional_injection_01() {


[2/3] jena git commit: Rename test cases and fix warnings (JENA-1497)

Posted by rv...@apache.org.
Rename test cases and fix warnings (JENA-1497)


Project: http://git-wip-us.apache.org/repos/asf/jena/repo
Commit: http://git-wip-us.apache.org/repos/asf/jena/commit/44683c42
Tree: http://git-wip-us.apache.org/repos/asf/jena/tree/44683c42
Diff: http://git-wip-us.apache.org/repos/asf/jena/diff/44683c42

Branch: refs/heads/master
Commit: 44683c424da9bea3b261151f7243524813917055
Parents: 411c103
Author: Rob Vesse <rv...@apache.org>
Authored: Mon Mar 5 10:30:58 2018 +0000
Committer: Rob Vesse <rv...@apache.org>
Committed: Mon Mar 5 10:30:58 2018 +0000

----------------------------------------------------------------------
 .../jena/query/ParameterizedSparqlString.java   |  2 +
 .../query/TestParameterizedSparqlString.java    | 82 ++++++++++----------
 2 files changed, 43 insertions(+), 41 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/jena/blob/44683c42/jena-arq/src/main/java/org/apache/jena/query/ParameterizedSparqlString.java
----------------------------------------------------------------------
diff --git a/jena-arq/src/main/java/org/apache/jena/query/ParameterizedSparqlString.java b/jena-arq/src/main/java/org/apache/jena/query/ParameterizedSparqlString.java
index 0e3e150..b02d8ca 100644
--- a/jena-arq/src/main/java/org/apache/jena/query/ParameterizedSparqlString.java
+++ b/jena-arq/src/main/java/org/apache/jena/query/ParameterizedSparqlString.java
@@ -1685,6 +1685,7 @@ public class ParameterizedSparqlString implements PrefixMapping {
             }
         }
 
+        @SuppressWarnings("unused")
         public boolean isInsideAltLiteral(int start, int stop) {
             Pair<Integer, String> pair = this.findBefore(start);
             if (pair == null)
@@ -1700,6 +1701,7 @@ public class ParameterizedSparqlString implements PrefixMapping {
             }
         }
 
+        @SuppressWarnings("unused")
         public boolean isBetweenLiterals(int start, int stop) {
             Pair<Integer, String> pairBefore = this.findBefore(start);
             if (pairBefore == null)

http://git-wip-us.apache.org/repos/asf/jena/blob/44683c42/jena-arq/src/test/java/org/apache/jena/query/TestParameterizedSparqlString.java
----------------------------------------------------------------------
diff --git a/jena-arq/src/test/java/org/apache/jena/query/TestParameterizedSparqlString.java b/jena-arq/src/test/java/org/apache/jena/query/TestParameterizedSparqlString.java
index 2f928e7..a7bcef3 100644
--- a/jena-arq/src/test/java/org/apache/jena/query/TestParameterizedSparqlString.java
+++ b/jena-arq/src/test/java/org/apache/jena/query/TestParameterizedSparqlString.java
@@ -66,6 +66,7 @@ public class TestParameterizedSparqlString {
         return query.asQuery();
     }
 
+    @SuppressWarnings("unused")
     private UpdateRequest testAsUpdate(ParameterizedSparqlString update) {
         return update.asUpdate();
     }
@@ -1383,7 +1384,7 @@ public class TestParameterizedSparqlString {
         ParameterizedSparqlString pss = new ParameterizedSparqlString(str);
         pss.setIri("var2", "hello> } ; DROP ALL ; INSERT DATA { <s> <p> <goodbye>");
 
-        UpdateRequest updates = pss.asUpdate();
+        pss.asUpdate();
         Assert.fail("Attempt to do SPARQL injection should result in an exception");
     }
 
@@ -1394,7 +1395,7 @@ public class TestParameterizedSparqlString {
         ParameterizedSparqlString pss = new ParameterizedSparqlString(str);
         pss.setIri("var2", "hello> } ; DROP ALL ; INSERT DATA { <s> <p> <goodbye");
 
-        UpdateRequest updates = pss.asUpdate();
+        pss.asUpdate();
         Assert.fail("Attempt to do SPARQL injection should result in an exception");
     }
 
@@ -1417,7 +1418,7 @@ public class TestParameterizedSparqlString {
         ParameterizedSparqlString pss = new ParameterizedSparqlString(str);
         pss.setIri("var2", "hello> . ?s ?p ?o");
 
-        Query q = pss.asQuery();
+        pss.asQuery();
         Assert.fail("Attempt to do SPARQL injection should result in an exception");
     }
 
@@ -1450,7 +1451,7 @@ public class TestParameterizedSparqlString {
         ParameterizedSparqlString pss = new ParameterizedSparqlString(str);
         pss.setLiteral("var", "hello' . } ; DROP ALL ; INSERT DATA { <s> <p> \"goodbye");
 
-        UpdateRequest updates = pss.asUpdate();
+        pss.asUpdate();
         Assert.fail("Attempt to do SPARQL injection should result in an exception");
     }
 
@@ -1462,7 +1463,7 @@ public class TestParameterizedSparqlString {
         ParameterizedSparqlString pss = new ParameterizedSparqlString(str);
         pss.setLiteral("var", " . } ; DROP ALL ; INSERT DATA { <s> <p> ");
 
-        UpdateRequest updates = pss.asUpdate();
+        pss.asUpdate();
         Assert.fail("Attempt to do SPARQL injection should result in an exception");
     }
 
@@ -1475,7 +1476,7 @@ public class TestParameterizedSparqlString {
         ParameterizedSparqlString pss = new ParameterizedSparqlString(str);
         pss.setLiteral("var", "' . } ; DROP ALL ; INSERT DATA { <s> <p> <o> }#");
 
-        UpdateRequest updates = pss.asUpdate();
+        pss.asUpdate();
         Assert.fail("Attempt to do SPARQL injection should result in an exception");
     }
 
@@ -1515,7 +1516,7 @@ public class TestParameterizedSparqlString {
         pss.setLiteral(first, "?" + second);
         pss.setLiteral(second, " . } ; DROP ALL ; INSERT DATA { <s> <p> ");
 
-        UpdateRequest updates = pss.asUpdate();
+        pss.asUpdate();
         Assert.fail("Attempt to do SPARQL injection should result in an exception");
     }
 
@@ -1528,7 +1529,7 @@ public class TestParameterizedSparqlString {
         ParameterizedSparqlString pss = new ParameterizedSparqlString(str);
         pss.setLiteral("var", " . } ; DROP ALL ; INSERT DATA { <s> <p> ");
 
-        UpdateRequest updates = pss.asUpdate();
+        pss.asUpdate();
         Assert.fail("Attempt to do SPARQL injection should result in an exception");
     }
 
@@ -1541,7 +1542,7 @@ public class TestParameterizedSparqlString {
         ParameterizedSparqlString pss = new ParameterizedSparqlString(str);
         pss.setLiteral("var", " . } ; DROP ALL ; INSERT DATA { <s> <p> ");
 
-        UpdateRequest updates = pss.asUpdate();
+        pss.asUpdate();
         Assert.fail("Attempt to do SPARQL injection should result in an exception");
     }
 
@@ -1589,36 +1590,10 @@ public class TestParameterizedSparqlString {
         pss.setLiteral(first, " ?" + second + " ");
         pss.setLiteral(second, " . } ; DROP ALL ; INSERT DATA { <s> <p> ");
 
-        UpdateRequest updates = pss.asUpdate();
+        pss.asUpdate();
         Assert.fail("Attempt to do SPARQL injection should result in an exception");
     }
     
-
-    
-    @Test
-    public void test_param_string_injection_16() {
-        String prefixes="PREFIX : <http://purl.bdrc.io/ontology/core/>\n" +
-                " PREFIX skos: <http://www.w3.org/2004/02/skos/core#>\n" +
-                " PREFIX text: <http://jena.apache.org/text#>\n" ;
-        HashMap<String,String> map=new HashMap<>();
-        map.put("L_name", "\"rgyud bla ma\"");
-        map.put("LG_name", "bo-x-ewts");
-        String test2=prefixes+ "select ?comment (GROUP_CONCAT(DISTINCT ?comment_type;  SEPARATOR=\" <>" +
-                "\") AS ?comment_types)  ?root_name\n" +
-                "where {\n" +
-                "    (?root ?score ?root_name) text:query ?L_name .\n" +
-                "    ?comment :workIsAbout ?root;\n" +
-                "             :workGenre ?g .\n" +
-                "    ?g skos:prefLabel ?comment_type .\n" +
-                "    FILTER (contains(?comment_type, \"commentary\" ))\n" +
-                "}\n" +
-                "group by ?comment ?root_name";
-        ParameterizedSparqlString queryStr2 = new ParameterizedSparqlString(test2);
-        queryStr2.setLiteral("L_name", map.get("L_name"),map.get("LG_name"));
-        System.out.println(queryStr2.toString());
-        Query q2=queryStr2.asQuery();
-    }
-
     @Test
     public void test_param_string_non_injection_01() {
         // This test checks that a legitimate injection of a literal to a
@@ -1652,6 +1627,31 @@ public class TestParameterizedSparqlString {
         queryStr.setLiteral("L_name", map.get("L_name"),map.get("LG_name"));        
         queryStr.asQuery();
     }
+    
+    
+    @Test
+    public void test_param_string_non_injection_03() {
+        String prefixes="PREFIX : <http://purl.bdrc.io/ontology/core/>\n" +
+                " PREFIX skos: <http://www.w3.org/2004/02/skos/core#>\n" +
+                " PREFIX text: <http://jena.apache.org/text#>\n" ;
+        HashMap<String,String> map=new HashMap<>();
+        map.put("L_name", "\"rgyud bla ma\"");
+        map.put("LG_name", "bo-x-ewts");
+        String test2=prefixes+ "select ?comment (GROUP_CONCAT(DISTINCT ?comment_type;  SEPARATOR=\" <>" +
+                "\") AS ?comment_types)  ?root_name\n" +
+                "where {\n" +
+                "    (?root ?score ?root_name) text:query ?L_name .\n" +
+                "    ?comment :workIsAbout ?root;\n" +
+                "             :workGenre ?g .\n" +
+                "    ?g skos:prefLabel ?comment_type .\n" +
+                "    FILTER (contains(?comment_type, \"commentary\" ))\n" +
+                "}\n" +
+                "group by ?comment ?root_name";
+        ParameterizedSparqlString queryStr2 = new ParameterizedSparqlString(test2);
+        queryStr2.setLiteral("L_name", map.get("L_name"),map.get("LG_name"));
+        System.out.println(queryStr2.toString());
+        queryStr2.asQuery();
+    }
 
     @Test(expected = ARQException.class)
     public void test_param_string_positional_injection_01() {
@@ -1660,7 +1660,7 @@ public class TestParameterizedSparqlString {
         ParameterizedSparqlString pss = new ParameterizedSparqlString(str);
         pss.setIri(0, "hello> } ; DROP ALL ; INSERT DATA { <s> <p> <goodbye>");
 
-        UpdateRequest updates = pss.asUpdate();
+        pss.asUpdate();
         Assert.fail("Attempt to do SPARQL injection should result in an exception");
     }
 
@@ -1671,7 +1671,7 @@ public class TestParameterizedSparqlString {
         ParameterizedSparqlString pss = new ParameterizedSparqlString(str);
         pss.setIri(0, "hello> } ; DROP ALL ; INSERT DATA { <s> <p> <goodbye");
 
-        UpdateRequest updates = pss.asUpdate();
+        pss.asUpdate();
         Assert.fail("Attempt to do SPARQL injection should result in an exception");
     }
 
@@ -1694,7 +1694,7 @@ public class TestParameterizedSparqlString {
         ParameterizedSparqlString pss = new ParameterizedSparqlString(str);
         pss.setIri(0, "hello> . ?s ?p ?o");
 
-        Query q = pss.asQuery();
+        pss.asQuery();
         Assert.fail("Attempt to do SPARQL injection should result in an exception");
     }
 
@@ -1811,7 +1811,7 @@ public class TestParameterizedSparqlString {
         ParameterizedSparqlString pss = new ParameterizedSparqlString(str);
         pss.setLiteral(0, " . } ; DROP ALL ; INSERT DATA { <s> <p> ");
 
-        UpdateRequest updates = pss.asUpdate();
+        pss.asUpdate();
         Assert.fail("Attempt to do SPARQL injection should result in an exception");
     }
 
@@ -1824,7 +1824,7 @@ public class TestParameterizedSparqlString {
         ParameterizedSparqlString pss = new ParameterizedSparqlString(str);
         pss.setLiteral(0, " . } ; DROP ALL ; INSERT DATA { <s> <p> ");
 
-        UpdateRequest updates = pss.asUpdate();
+        pss.asUpdate();
         Assert.fail("Attempt to do SPARQL injection should result in an exception");
     }
 


[3/3] jena git commit: Merge branch 'JENA-1497'

Posted by rv...@apache.org.
Merge branch 'JENA-1497'

This closes #370


Project: http://git-wip-us.apache.org/repos/asf/jena/repo
Commit: http://git-wip-us.apache.org/repos/asf/jena/commit/2e732b48
Tree: http://git-wip-us.apache.org/repos/asf/jena/tree/2e732b48
Diff: http://git-wip-us.apache.org/repos/asf/jena/diff/2e732b48

Branch: refs/heads/master
Commit: 2e732b48500f5856d9b4ba639960b8bd8baa8a1a
Parents: b75fcce 44683c4
Author: Rob Vesse <rv...@apache.org>
Authored: Mon Mar 5 14:54:30 2018 +0000
Committer: Rob Vesse <rv...@apache.org>
Committed: Mon Mar 5 14:54:30 2018 +0000

----------------------------------------------------------------------
 .../jena/query/ParameterizedSparqlString.java   | 32 +++++---
 .../query/TestParameterizedSparqlString.java    | 81 ++++++++++++++++----
 2 files changed, 88 insertions(+), 25 deletions(-)
----------------------------------------------------------------------