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 2013/03/28 23:40:18 UTC

svn commit: r1462333 - in /jena/trunk/jena-arq/src: main/java/com/hp/hpl/jena/query/ParameterizedSparqlString.java test/java/com/hp/hpl/jena/query/TestParameterizedSparqlString.java

Author: rvesse
Date: Thu Mar 28 22:40:17 2013
New Revision: 1462333

URL: http://svn.apache.org/r1462333
Log:
Improve protection against SPARQL injection in ParameterizedSparqlString, adds additional test cases to cover other possible attacks

Modified:
    jena/trunk/jena-arq/src/main/java/com/hp/hpl/jena/query/ParameterizedSparqlString.java
    jena/trunk/jena-arq/src/test/java/com/hp/hpl/jena/query/TestParameterizedSparqlString.java

Modified: jena/trunk/jena-arq/src/main/java/com/hp/hpl/jena/query/ParameterizedSparqlString.java
URL: http://svn.apache.org/viewvc/jena/trunk/jena-arq/src/main/java/com/hp/hpl/jena/query/ParameterizedSparqlString.java?rev=1462333&r1=1462332&r2=1462333&view=diff
==============================================================================
--- jena/trunk/jena-arq/src/main/java/com/hp/hpl/jena/query/ParameterizedSparqlString.java (original)
+++ jena/trunk/jena-arq/src/main/java/com/hp/hpl/jena/query/ParameterizedSparqlString.java Thu Mar 28 22:40:17 2013
@@ -1149,6 +1149,28 @@ public class ParameterizedSparqlString i
         this.params.clear();
         this.positionalParams.clear();
     }
+    
+    /**
+     * Helper method which checks whether it is safe to inject to a variable parameter the given value
+     * @param command Current command string
+     * @param var Variable
+     * @param n Value to inject
+     * @throws ARQException Thrown if not safe to inject, error message will describe why it is unsafe to inject
+     */
+    protected void validateSafeToInject(String command, String var, Node n) throws ARQException {
+        // Looks for the known injection attack vectors and throws an error if any
+        // are encountered
+        
+        // A ?var surrounded by " or ' where the variable is a literal is an attack vector
+        Pattern p = Pattern.compile("\"[?$]" + var + "\"|'[?$]" + var + "'");
+
+        if (p.matcher(command).find() && n.isLiteral()) {
+            throw new ARQException(
+                    "Command string is vunerable to injection attack, variable ?"
+                            + var
+                            + " appears surrounded directly by quotes and is bound to a literal which provides a SPARQL injection attack vector");
+        }
+    }
 
     /**
      * This method is where the actual work happens, the original command text
@@ -1169,31 +1191,19 @@ public class ParameterizedSparqlString i
         String command = this.cmd.toString();
         Pattern p;
 
-        // Before we do anything scan for obvious things that can lead to SPARQL
-        // injection attacks
-        // The text "?var" where ?var is bound to a literal is an injection
-        // attack
-        for (Entry<String, Node> entry : this.params.entrySet()) {
-            p = Pattern.compile("\"[?$]" + entry.getKey() + "\"");
-
-            if (p.matcher(command).find() && entry.getValue().isLiteral()) {
-                throw new ARQException(
-                        "Command string is vunerable to injection attack, variable ?"
-                                + entry.getKey()
-                                + " appears surrounded by quotes and is bound to a literal which provides a SPARQL injection attack vector");
-            }
-        }
-
         // Go ahead and inject Variable Parameters
         SerializationContext context = new SerializationContext(this.prefixes);
         context.setBaseIRI(this.baseUri);
         Iterator<String> vars = this.params.keySet().iterator();
         while (vars.hasNext()) {
             String var = vars.next();
+            Node n = this.params.get(var);
+            if (n == null) continue;
+            this.validateSafeToInject(command, var, n);
 
             p = Pattern.compile("([?$]" + var + ")([^\\w]|$)");
             command = p.matcher(command).replaceAll(
-                    Matcher.quoteReplacement(FmtUtils.stringForNode(this.params.get(var), context)) + "$2");
+                    Matcher.quoteReplacement(FmtUtils.stringForNode(n, context)) + "$2");
         }
 
         // Then inject Positional Parameters

Modified: jena/trunk/jena-arq/src/test/java/com/hp/hpl/jena/query/TestParameterizedSparqlString.java
URL: http://svn.apache.org/viewvc/jena/trunk/jena-arq/src/test/java/com/hp/hpl/jena/query/TestParameterizedSparqlString.java?rev=1462333&r1=1462332&r2=1462333&view=diff
==============================================================================
--- jena/trunk/jena-arq/src/test/java/com/hp/hpl/jena/query/TestParameterizedSparqlString.java (original)
+++ jena/trunk/jena-arq/src/test/java/com/hp/hpl/jena/query/TestParameterizedSparqlString.java Thu Mar 28 22:40:17 2013
@@ -1268,101 +1268,211 @@ public class TestParameterizedSparqlStri
         Assert.assertEquals("SELECT * WHERE { <http://example.org> <http://predicate> \"test\", ?o . }", query.toString());
     }
 
-    @Test(expected=ARQException.class)
+    @Test(expected = ARQException.class)
     public void test_param_string_injection_01() {
+        // This injection is prevented by forbidding the > character in URIs
         String str = "PREFIX : <http://example/>\nINSERT DATA { <s> <p> ?var2 . }";
         ParameterizedSparqlString pss = new ParameterizedSparqlString(str);
         pss.setIri("var2", "hello> } ; DROP ALL ; INSERT DATA { <s> <p> <goodbye>");
-        
+
         UpdateRequest updates = pss.asUpdate();
         Assert.fail("Attempt to do SPARQL injection should result in an exception");
     }
-    
-    @Test(expected=ARQException.class)
+
+    @Test(expected = ARQException.class)
     public void test_param_string_injection_02() {
+        // This injection is prevented by forbidding the > character in URIs
         String str = "PREFIX : <http://example/>\nINSERT DATA { <s> <p> ?var2 . }";
         ParameterizedSparqlString pss = new ParameterizedSparqlString(str);
         pss.setIri("var2", "hello> } ; DROP ALL ; INSERT DATA { <s> <p> <goodbye");
-        
+
         UpdateRequest updates = pss.asUpdate();
         Assert.fail("Attempt to do SPARQL injection should result in an exception");
     }
-    
+
     @Test
     public void test_param_string_injection_03() {
+        // This injection attempt results in a valid update but a failed
+        // injection
         String str = "PREFIX : <http://example/>\nINSERT DATA { <s> <p> ?var2 . }";
         ParameterizedSparqlString pss = new ParameterizedSparqlString(str);
         pss.setLiteral("var2", "hello\" } ; DROP ALL ; INSERT DATA { <s> <p> <goodbye>");
-        
+
         UpdateRequest updates = pss.asUpdate();
         Assert.assertEquals(1, updates.getOperations().size());
     }
-    
-    @Test(expected=ARQException.class)
+
+    @Test(expected = ARQException.class)
     public void test_param_string_injection_04() {
+        // This injection is prevented by forbidding the > character in URIs
         String str = "PREFIX : <http://example/>\nSELECT * WHERE { <s> <p> ?var2 . }";
         ParameterizedSparqlString pss = new ParameterizedSparqlString(str);
         pss.setIri("var2", "hello> . ?s ?p ?o");
-        
+
         Query q = pss.asQuery();
         Assert.fail("Attempt to do SPARQL injection should result in an exception");
     }
-    
+
     @Test
     public void test_param_string_injection_05() {
+        // This injection attempt results in a valid query but a failed
+        // injection
         String str = "PREFIX : <http://example/>\nSELECT * WHERE { <s> <p> ?var2 . }";
         ParameterizedSparqlString pss = new ParameterizedSparqlString(str);
         pss.setLiteral("var2", "hello\" . ?s ?p ?o");
-        
+
         Query q = pss.asQuery();
         Element el = q.getQueryPattern();
         if (el instanceof ElementTriplesBlock) {
-            Assert.assertEquals(1, ((ElementTriplesBlock)q.getQueryPattern()).getPattern().size());
+            Assert.assertEquals(1, ((ElementTriplesBlock) q.getQueryPattern()).getPattern().size());
         } else if (el instanceof ElementGroup) {
-            Assert.assertEquals(1, ((ElementGroup)el).getElements().size());
-            el = ((ElementGroup)el).getElements().get(0);
+            Assert.assertEquals(1, ((ElementGroup) el).getElements().size());
+            el = ((ElementGroup) el).getElements().get(0);
             if (el instanceof ElementTriplesBlock) {
-                Assert.assertEquals(1, ((ElementTriplesBlock)el).getPattern().size());
+                Assert.assertEquals(1, ((ElementTriplesBlock) el).getPattern().size());
             }
         }
     }
-    
-    @Test(expected=QueryParseException.class)
+
+    @Test(expected = ARQException.class)
     public void test_param_string_injection_06() {
-        String str = "PREFIX : <http://example/>\nINSERT DATA { <s> <p> '?var' }" ;
+        // This injection attempt is prevented by forbidding injection to a variable
+        // parameter immediatedly surrounded by quotes
+        String str = "PREFIX : <http://example/>\nINSERT DATA { <s> <p> '?var' }";
         ParameterizedSparqlString pss = new ParameterizedSparqlString(str);
         pss.setLiteral("var", "hello' . } ; DROP ALL ; INSERT DATA { <s> <p> \"goodbye");
-        
+
         UpdateRequest updates = pss.asUpdate();
         Assert.fail("Attempt to do SPARQL injection should result in an exception");
     }
-    
-    @Test(expected=ARQException.class)
+
+    @Test(expected = ARQException.class)
     public void test_param_string_injection_07() {
-        String str = "PREFIX : <http://example/>\nINSERT DATA { <s> <p> \"?var\" }" ;
+        // This injection attempt is prevented by forbidding injection of
+        // variable parameters immediately surrounded by quotes
+        String str = "PREFIX : <http://example/>\nINSERT DATA { <s> <p> \"?var\" }";
         ParameterizedSparqlString pss = new ParameterizedSparqlString(str);
         pss.setLiteral("var", " . } ; DROP ALL ; INSERT DATA { <s> <p> ");
+
+        UpdateRequest updates = pss.asUpdate();
+        Assert.fail("Attempt to do SPARQL injection should result in an exception");
+    }
+    
+    @Test(expected = ARQException.class)
+    public void test_param_string_injection_08() {
+        // This injection attempt results in an invalid SPARQL update because you end up with
+        // a double quoted literal inside a single quoted literal
+        String str = "PREFIX : <http://example/>\nINSERT DATA { <s> <p> '?var' }";
+        ParameterizedSparqlString pss = new ParameterizedSparqlString(str);
+        pss.setLiteral("var", "' . } ; DROP ALL ; INSERT DATA { <s> <p> <o> }#");
+
+        UpdateRequest updates = pss.asUpdate();
+        Assert.fail("Attempt to do SPARQL injection should result in an exception");
+    }
+
+    @Test
+    public void test_param_string_injection_09() {
+        // This injection attempt using comments results in a valid SPARQL update but a failed
+        // injection because the attempt to use comments ends up being a valid string
+        // literal within quotes
+        String str = "PREFIX : <http://example/>\nINSERT DATA { <s> <p> ?var }";
+        ParameterizedSparqlString pss = new ParameterizedSparqlString(str);
+        pss.setLiteral("var", "\" . } ; DROP ALL ; INSERT DATA { <s> <p> <o> }#");
+
+        UpdateRequest updates = pss.asUpdate();
+        Assert.assertEquals(1, updates.getOperations().size());
+    }
+    
+    @Test(expected=ARQException.class)
+    public void test_param_string_injection_10() {
+        // This injection attempt tries to chain together injections to achieve an attack, the first
+        // injection appears innocuous and is an attempt to set up an actual injection vector
+        // The injection is prevented because a ?var directly surrounded by quotes is always flagged as
+        // subject to injection because pre-injection validation happens before each variable is injected
+        String str = "PREFIX : <http://example/>\nINSERT DATA { <s> <p> ?var }";
+        ParameterizedSparqlString pss = new ParameterizedSparqlString(str);
+        pss.setLiteral("var", "a");
+        pss.setLiteral("var2", "b");
+        
+        // Figure out which variable will be injected first
+        String first = pss.getVars().next();
+        String second = first.equals("var") ? "var2" : "var";
         
+        pss.setLiteral(first, "?" + second);
+        pss.setLiteral(second, " . } ; DROP ALL ; INSERT DATA { <s> <p> ");
+
         UpdateRequest updates = pss.asUpdate();
         Assert.fail("Attempt to do SPARQL injection should result in an exception");
     }
     
     @Test
     public void test_param_string_injection_permitted_01() {
-        String str = "PREFIX : <http://example/>\nINSERT DATA { <s> <p> \" ?var \" }" ;
+        // This is a case where we cannot detect the different between a valid
+        // parameterized
+        // string and one that is subject to injection
+        String str = "PREFIX : <http://example/>\nINSERT DATA { <s> <p> \" ?var \" }";
         ParameterizedSparqlString pss = new ParameterizedSparqlString(str);
         pss.setLiteral("var", " . } ; DROP ALL ; INSERT DATA { <s> <p> ");
-        
+
         UpdateRequest updates = pss.asUpdate();
         Assert.assertEquals(3, updates.getOperations().size());
     }
-    
+
     @Test
     public void test_param_string_injection_permitted_02() {
-        String str = "PREFIX : <http://example/>\nINSERT DATA { <s> <p> \"some text ?var other text\" }" ;
+        // This is a case where we cannot detect the different between a valid
+        // parameterized
+        // string and one that is subject to injection
+        String str = "PREFIX : <http://example/>\nINSERT DATA { <s> <p> \"some text ?var other text\" }";
         ParameterizedSparqlString pss = new ParameterizedSparqlString(str);
         pss.setLiteral("var", " . } ; DROP ALL ; INSERT DATA { <s> <p> ");
+
+        UpdateRequest updates = pss.asUpdate();
+        Assert.assertEquals(3, updates.getOperations().size());
+    }
+    
+    @Test
+    public void test_param_string_injection_permitted_03() {
+        // This is a case where we cannot detect the different between a valid
+        // parameterized string and one that is subject to injection
+        String str = "PREFIX : <http://example/>\nINSERT DATA { <s> <p> ' ?var ' }";
+        ParameterizedSparqlString pss = new ParameterizedSparqlString(str);
+        pss.setLiteral("var", "' . } ; DROP ALL ; INSERT DATA { <s> <p> <o> }#");
+
+        UpdateRequest updates = pss.asUpdate();
+        Assert.assertEquals(3, updates.getOperations().size());
+    }
+
+    @Test
+    public void test_param_string_injection_permitted_04() {
+        // This is a case where we cannot detect the different between a valid
+        // parameterized string and one that is subject to injection
+        String str = "PREFIX : <http://example/>\nINSERT DATA { <s> <p> 'some text ?var other text' }";
+        ParameterizedSparqlString pss = new ParameterizedSparqlString(str);
+        pss.setLiteral("var", "' . } ; DROP ALL ; INSERT DATA { <s> <p> <o> }#");
+
+        UpdateRequest updates = pss.asUpdate();
+        Assert.assertEquals(3, updates.getOperations().size());
+    }
+    
+    @Test
+    public void test_param_string_injection_permitted_05() {
+        // This injection attempt tries to chain together injections to achieve an attack, the first
+        // injection appears innocuous and is an attempt to set up an actual injection vector
+        // The injection is prevented because a ?var directly surrounded by quotes is always flagged as
+        // subject to injection because pre-injection validation happens before each variable is injected
+        String str = "PREFIX : <http://example/>\nINSERT DATA { <s> <p> ?var }";
+        ParameterizedSparqlString pss = new ParameterizedSparqlString(str);
+        pss.setLiteral("var", "a");
+        pss.setLiteral("var2", "b");
+        
+        // Figure out which variable will be injected first
+        String first = pss.getVars().next();
+        String second = first.equals("var") ? "var2" : "var";
         
+        pss.setLiteral(first, " ?" + second + " ");
+        pss.setLiteral(second, " . } ; DROP ALL ; INSERT DATA { <s> <p> ");
+
         UpdateRequest updates = pss.asUpdate();
         Assert.assertEquals(3, updates.getOperations().size());
     }