Update for collection flow and add more tests

2026-04-29 02:35:15 +02:00 · 2021-06-08 11:33:51 +01:00
parent 8f89d748fe
commit df74a142dd
2 changed files with 161 additions and 28 deletions
--- a/java/ql/src/semmle/code/java/frameworks/spring/SpringHttp.qll
+++ b/java/ql/src/semmle/code/java/frameworks/spring/SpringHttp.qll
@@ -67,26 +67,36 @@ private class SpringHttpFlowStep extends SinkModelCsv {
    row =
      [
        //"package;type;overrides;name;signature;ext;inputspec;outputspec;kind",
-        "org.springframework.http;HttpEntity;true;HttpEntity;(T);;Argument[0];Argument[-1];taint",
-        "org.springframework.http;HttpEntity;true;HttpEntity;(T,MultiValueMap<String,String>);;Argument[0];Argument[-1];taint",
-        // Constructor with signature (MultiValueMap<String,String>) dependant on collection flow
+        "org.springframework.http;HttpEntity;true;HttpEntity;(Object);;Argument[0];Argument[-1];taint",
+        "org.springframework.http;HttpEntity;true;HttpEntity;(Object,MultiValueMap);;Argument[0];Argument[-1];taint",
+        "org.springframework.http;HttpEntity;true;HttpEntity;(Object,MultiValueMap);;MapKey of Argument[1];Argument[-1];taint",
+        "org.springframework.http;HttpEntity;true;HttpEntity;(Object,MultiValueMap);;Element of MapValue of Argument[1];Argument[-1];taint",
+        "org.springframework.http;HttpEntity;true;HttpEntity;(MultiValueMap);;MapKey of Argument[0];Argument[-1];taint",
+        "org.springframework.http;HttpEntity;true;HttpEntity;(MultiValueMap);;Element of MapValue of Argument[0];Argument[-1];taint",
        "org.springframework.http;HttpEntity;true;getBody;;;Argument[-1];ReturnValue;taint",
        "org.springframework.http;HttpEntity;true;getHeaders;;;Argument[-1];ReturnValue;taint",
-        "org.springframework.http;ResponseEntity;true;ResponseEntity;(T,HttpStatus);;Argument[0];Argument[-1];taint",
-        "org.springframework.http;ResponseEntity;true;ResponseEntity;(T,MultiValueMap<String,String>,HttpStatus);;Argument[0];Argument[-1];taint",
-        "org.springframework.http;ResponseEntity;true;ResponseEntity;(T,MultiValueMap<String,String>,int);;Argument[0];Argument[-1];taint",
-        "org.springframework.http;ResponseEntity;true;of;(Optional<T>);;Argument[0];ReturnValue;taint",
-        "org.springframework.http;ResponseEntity;true;ok;(T);;Argument[0];ReturnValue;taint",
+        "org.springframework.http;ResponseEntity;true;ResponseEntity;(Object,HttpStatus);;Argument[0];Argument[-1];taint",
+        "org.springframework.http;ResponseEntity;true;ResponseEntity;(Object,MultiValueMap,HttpStatus);;Argument[0];Argument[-1];taint",
+        "org.springframework.http;ResponseEntity;true;ResponseEntity;(Object,MultiValueMap,HttpStatus);;MapKey of Argument[1];Argument[-1];taint",
+        "org.springframework.http;ResponseEntity;true;ResponseEntity;(Object,MultiValueMap,HttpStatus);;Element of MapValue of Argument[1];Argument[-1];taint",
+        "org.springframework.http;ResponseEntity;true;ResponseEntity;(MultiValueMap,HttpStatus);;MapKey of Argument[0];Argument[-1];taint",
+        "org.springframework.http;ResponseEntity;true;ResponseEntity;(MultiValueMap,HttpStatus);;Element of MapValue of Argument[0];Argument[-1];taint",
+        "org.springframework.http;ResponseEntity;true;ResponseEntity;(Object,MultiValueMap,int);;Argument[0];Argument[-1];taint",
+        "org.springframework.http;ResponseEntity;true;ResponseEntity;(Object,MultiValueMap,int);;MapKey of Argument[1];Argument[-1];taint",
+        "org.springframework.http;ResponseEntity;true;ResponseEntity;(Object,MultiValueMap,int);;Element of MapValue of Argument[1];Argument[-1];taint",
+        "org.springframework.http;ResponseEntity;true;of;(Optional);;Argument[0];ReturnValue;taint",
+        "org.springframework.http;ResponseEntity;true;ok;(Object);;Argument[0];ReturnValue;taint",
        "org.springframework.http;ResponseEntity;true;created;(URI);;Argument[0];ReturnValue;taint",
        "org.springframework.http;ResponseEntity<>$BodyBuilder;true;contentLength;(long);;Argument[-1];ReturnValue;value",
        "org.springframework.http;ResponseEntity<>$BodyBuilder;true;contentType;(MediaType);;Argument[-1];ReturnValue;value",
-        "org.springframework.http;ResponseEntity<>$BodyBuilder;true;body;(T);;Argument[-1..0];ReturnValue;taint",
+        "org.springframework.http;ResponseEntity<>$BodyBuilder;true;body;(Object);;Argument[-1..0];ReturnValue;taint",
        "org.springframework.http;ResponseEntity<>$HeadersBuilder;true;allow;(HttpMethod[]);;Argument[-1];ReturnValue;value",
        "org.springframework.http;ResponseEntity<>$HeadersBuilder;true;eTag;(String);;Argument[-1];ReturnValue;value",
        "org.springframework.http;ResponseEntity<>$HeadersBuilder;true;eTag;(String);;Argument[0];Argument[-1];taint",
        "org.springframework.http;ResponseEntity<>$HeadersBuilder;true;header;(String,String[]);;Argument[-1];ReturnValue;value",
-        "org.springframework.http;ResponseEntity<>$HeadersBuilder;true;header;(String,String[]);;Argument[0..1];Argument[-1];taint",
-        "org.springframework.http;ResponseEntity<>$HeadersBuilder;true;headers;(Consumer<HttpHeader>);;Argument[-1];ReturnValue;value",
+        "org.springframework.http;ResponseEntity<>$HeadersBuilder;true;header;(String,String[]);;Argument[0];Argument[-1];taint",
+        "org.springframework.http;ResponseEntity<>$HeadersBuilder;true;header;(String,String[]);;ArrayElement of Argument[1];Argument[-1];taint",
+        "org.springframework.http;ResponseEntity<>$HeadersBuilder;true;headers;(Consumer);;Argument[-1];ReturnValue;value",
        "org.springframework.http;ResponseEntity<>$HeadersBuilder;true;headers;(HttpHeaders);;Argument[-1];ReturnValue;value",
        "org.springframework.http;ResponseEntity<>$HeadersBuilder;true;headers;(HttpHeaders);;Argument[0];Argument[-1];taint",
        "org.springframework.http;ResponseEntity<>$HeadersBuilder;true;lastModified;;;Argument[-1];ReturnValue;value",
@@ -95,29 +105,38 @@ private class SpringHttpFlowStep extends SinkModelCsv {
        "org.springframework.http;ResponseEntity<>$HeadersBuilder;true;varyBy;(String[]);;Argument[-1];ReturnValue;value",
        "org.springframework.http;ResponseEntity<>$HeadersBuilder;true;build;();;Argument[-1];ReturnValue;taint",
        "org.springframework.http;RequestEntity;true;getUrl;();;Argument[-1];ReturnValue;taint",
-        "org.springframework.http;HttpHeaders;true;get;(Object);;Argument[-1];ReturnValue;taint", // Returns List<String>
-        "org.springframework.http;HttpHeaders;true;getAccessControlAllowHeaders;();;Argument[-1];ReturnValue;taint", // Returns List<String>
+        "org.springframework.http;HttpHeaders;true;HttpHeaders;(MultiValueMap);;MapKey of Argument[0];Argument[-1];taint",
+        "org.springframework.http;HttpHeaders;true;HttpHeaders;(MultiValueMap);;Element of MapValue of Argument[0];Argument[-1];taint",
+        "org.springframework.http;HttpHeaders;true;get;(Object);;Argument[-1];Element of ReturnValue;taint",
+        "org.springframework.http;HttpHeaders;true;getAccessControlAllowHeaders;();;Argument[-1];Element of ReturnValue;taint",
        "org.springframework.http;HttpHeaders;true;getAccessControlAllowOrigin;();;Argument[-1];ReturnValue;taint",
-        "org.springframework.http;HttpHeaders;true;getAccessControlExposeHeaders;();;Argument[-1];ReturnValue;taint", // Returns List<String>
-        "org.springframework.http;HttpHeaders;true;getAccessControlRequestHeaders;();;Argument[-1];ReturnValue;taint", // Returns List<String>
+        "org.springframework.http;HttpHeaders;true;getAccessControlExposeHeaders;();;Argument[-1];Element of ReturnValue;taint",
+        "org.springframework.http;HttpHeaders;true;getAccessControlRequestHeaders;();;Argument[-1];Element of ReturnValue;taint",
        "org.springframework.http;HttpHeaders;true;getCacheControl;();;Argument[-1];ReturnValue;taint",
-        "org.springframework.http;HttpHeaders;true;getConnection;();;Argument[-1];ReturnValue;taint", // Returns List<String>
+        "org.springframework.http;HttpHeaders;true;getConnection;();;Argument[-1];Element of ReturnValue;taint",
        "org.springframework.http;HttpHeaders;true;getETag;();;Argument[-1];ReturnValue;taint",
-        "org.springframework.http;HttpHeaders;true;getETagValuesAsList;(String);;Argument[-1];ReturnValue;taint", // Returns List<String>
+        "org.springframework.http;HttpHeaders;true;getETagValuesAsList;(String);;Element of Argument[-1];ReturnValue;taint",
        "org.springframework.http;HttpHeaders;true;getFieldValues;(String);;Argument[-1];ReturnValue;taint",
        "org.springframework.http;HttpHeaders;true;getFirst;(String);;Argument[-1];ReturnValue;taint",
-        "org.springframework.http;HttpHeaders;true;getIfMatch;();;Argument[-1];ReturnValue;taint", // Returns List<String>
-        "org.springframework.http;HttpHeaders;true;getIfNoneMatch;();;Argument[-1];ReturnValue;taint", // Returns List<String>
+        "org.springframework.http;HttpHeaders;true;getIfMatch;();;Argument[-1];Element of ReturnValue;taint",
+        "org.springframework.http;HttpHeaders;true;getIfNoneMatch;();;Argument[-1];Element of ReturnValue;taint",
+        "org.springframework.http;HttpHeaders;true;getHost;();;Argument[-1];ReturnValue;taint",
        "org.springframework.http;HttpHeaders;true;getLocation;();;Argument[-1];ReturnValue;taint",
-        "org.springframework.http;HttpHeaders;true;getOrEmpty;(Object);;Argument[-1];ReturnValue;taint", // Returns List<String>
+        "org.springframework.http;HttpHeaders;true;getOrEmpty;(Object);;Argument[-1];Element of ReturnValue;taint",
        "org.springframework.http;HttpHeaders;true;getOrigin;();;Argument[-1];ReturnValue;taint",
        "org.springframework.http;HttpHeaders;true;getPragma;();;Argument[-1];ReturnValue;taint",
        "org.springframework.http;HttpHeaders;true;getUpgrade;();;Argument[-1];ReturnValue;taint",
-        "org.springframework.http;HttpHeaders;true;getValuesAsList;(String);;Argument[-1];ReturnValue;taint", // Returns List<String>
-        "org.springframework.http;HttpHeaders;true;getVary;();;Argument[-1];ReturnValue;taint", // Returns List<String>
+        "org.springframework.http;HttpHeaders;true;getValuesAsList;(String);;Argument[-1];Element of ReturnValue;taint",
+        "org.springframework.http;HttpHeaders;true;getVary;();;Argument[-1];Element of ReturnValue;taint",
        "org.springframework.http;HttpHeaders;true;add;(String,String);;Argument[0..1];Argument[-1];taint",
        "org.springframework.http;HttpHeaders;true;set;(String,String);;Argument[0..1];Argument[-1];taint",
-        "org.springframework.http;HttpHeaders;true;addAll;;;Argument[0..1];Argument[-1];taint" // dependant on collection flow
+        "org.springframework.http;HttpHeaders;true;addAll;(MultiValueMap);;MapKey of Argument[0];Argument[-1];taint",
+        "org.springframework.http;HttpHeaders;true;addAll;(MultiValueMap);;Element of MapValue of Argument[0];Argument[-1];taint",
+        "org.springframework.http;HttpHeaders;true;addAll;(String,List);;Argument[0];Argument[-1];taint",
+        "org.springframework.http;HttpHeaders;true;addAll;(String,List);;Element of Argument[1];Argument[-1];taint",
+        "org.springframework.http;HttpHeaders;true;formatHeaders;(MultiValueMap);;MapKey of Argument[0];ReturnValue;taint",
+        "org.springframework.http;HttpHeaders;true;formatHeaders;(MultiValueMap);;Element of MapValue of Argument[0];ReturnValue;taint",
+        "org.springframework.http;HttpHeaders;true;encodeBasicAuth;(String,String,Charset);;Argument[0..1];ReturnValue;taint"
      ]
  }
 }
--- a/java/ql/test/library-tests/frameworks/spring/TestHttp.java
+++ b/java/ql/test/library-tests/frameworks/spring/TestHttp.java
@@ -2,9 +2,11 @@ import org.springframework.http.HttpEntity;
 import org.springframework.http.ResponseEntity;
 import org.springframework.http.RequestEntity;
 import org.springframework.http.HttpHeaders;
+import org.springframework.http.HttpStatus;
 import org.springframework.util.MultiValueMap;
 import org.springframework.util.LinkedMultiValueMap;
 import java.util.Optional;
+import java.util.List;

 class TestHttp {
    static <T> T taint() { return null; }
@@ -14,12 +16,16 @@ class TestHttp {
        String x = taint();
        sink(new HttpEntity(x)); // $hasTaintFlow

-        MultiValueMap<String,String> m = new LinkedMultiValueMap();
-        sink(new HttpEntity(x, m)); // $hasTaintFlow
+        MultiValueMap<String,String> m1 = new LinkedMultiValueMap();
+        sink(new HttpEntity(x, m1)); // $hasTaintFlow

-        m.add("a", taint());
-        sink(new HttpEntity("a", m)); // $ MISSING:hasTaintFlow
-        sink(new HttpEntity<String>(m)); // $ MISSING:hasTaintFlow
+        m1.add("a", taint());
+        sink(new HttpEntity("a", m1)); // $ MISSING:hasTaintFlow
+        sink(new HttpEntity<String>(m1)); // $ MISSING:hasTaintFlow
+
+        MultiValueMap<String,String> m2 = new LinkedMultiValueMap();
+        m2.add(taint(), "a");
+        sink(new HttpEntity<String>(m2)); // $ MISSING:hasTaintFlow

        HttpEntity<String> ent = taint();
        sink(ent.getBody()); // $hasTaintFlow
@@ -45,4 +51,112 @@ class TestHttp {
        sink(ResponseEntity.status(200).location(taint()).lastModified(10000000).build()); // $hasTaintFlow
        sink(ResponseEntity.status(200).varyBy(x).build()); 
    }
+
+    void test3() {
+        String x = taint();
+
+        MultiValueMap<String,String> m1 = new LinkedMultiValueMap();
+        sink(new ResponseEntity(x, HttpStatus.ACCEPTED)); // $hasTaintFlow
+        sink(new ResponseEntity(x, m1, HttpStatus.ACCEPTED)); // $hasTaintFlow
+        sink(new ResponseEntity(x, m1, 200)); // $hasTaintFlow
+
+        m1.add("a", taint());
+        sink(new ResponseEntity("a", m1, HttpStatus.ACCEPTED)); // $ MISSING:hasTaintFlow
+        sink(new ResponseEntity<String>(m1, HttpStatus.ACCEPTED)); // $ MISSING:hasTaintFlow
+        sink(new ResponseEntity("a", m1, 200)); // $ MISSING:hasTaintFlow
+
+        MultiValueMap<String,String> m2 = new LinkedMultiValueMap();
+        m2.add(taint(), "a");
+        sink(new ResponseEntity("a", m2, HttpStatus.ACCEPTED)); // $ MISSING:hasTaintFlow
+        sink(new ResponseEntity<String>(m2, HttpStatus.ACCEPTED)); // $ MISSING:hasTaintFlow
+        sink(new ResponseEntity("a", m2, 200)); // $ MISSING:hasTaintFlow
+
+        ResponseEntity<String> ent = taint();
+        sink(ent.getBody()); // $hasTaintFlow
+        sink(ent.getHeaders()); // $hasTaintFlow
+    }
+
+    void test4() {
+        MultiValueMap<String,String> m1 = new LinkedMultiValueMap();
+        m1.add("a", taint());
+        sink(new HttpHeaders(m1)); // $ MISSING:hasTaintFlow
+
+        MultiValueMap<String,String> m2 = new LinkedMultiValueMap();
+        m2.add(taint(), "a");
+        sink(new HttpHeaders(m2)); // $ MISSING:hasTaintFlow
+
+        HttpHeaders h1 = new HttpHeaders();
+        h1.add(taint(), "a"); 
+        sink(h1); // $hasTaintFlow
+
+        HttpHeaders h2 = new HttpHeaders();
+        h2.add("a", taint()); 
+        sink(h2); // $hasTaintFlow
+
+        HttpHeaders h3 = new HttpHeaders();
+        h3.addAll(m1); 
+        sink(h3); // $ MISSING:hasTaintFlow
+
+        HttpHeaders h4 = new HttpHeaders();
+        h4.addAll(m2); 
+        sink(h4); // $ MISSING:hasTaintFlow
+
+        HttpHeaders h5 = new HttpHeaders();
+        h5.addAll(taint(), List.of()); 
+        sink(h5); // $hasTaintFlow
+
+        HttpHeaders h6 = new HttpHeaders();
+        h6.addAll("a", List.of(taint())); 
+        sink(h6); // $hasTaintFlow
+
+        sink(HttpHeaders.formatHeaders(m1)); // $ MISSING:hasTaintFlow
+        sink(HttpHeaders.formatHeaders(m2)); // $ MISSING:hasTaintFlow
+
+        sink(HttpHeaders.encodeBasicAuth(taint(), "a", null)); // $hasTaintFlow
+        sink(HttpHeaders.encodeBasicAuth("a", taint(), null)); // $hasTaintFlow
+    }
+
+    void test5() {
+        HttpHeaders h = taint();
+        
+        sink(h.get(null).get(0)); // $hasTaintFlow
+        sink(h.getAccept().get(0));
+        sink(h.getAcceptCharset().get(0));
+        sink(h.getAcceptLanguage().get(0));
+        sink(h.getAcceptLanguageAsLocales().get(0));
+        sink(h.getAccessControlAllowCredentials());
+        sink(h.getAccessControlAllowHeaders().get(0)); // $hasTaintFlow
+        sink(h.getAccessControlAllowMethods().get(0));
+        sink(h.getAccessControlAllowOrigin()); // $hasTaintFlow
+        sink(h.getAccessControlExposeHeaders().get(0)); // $hasTaintFlow
+        sink(h.getAccessControlMaxAge());
+        sink(h.getAccessControlRequestHeaders().get(0)); // $hasTaintFlow
+        sink(h.getAccessControlRequestMethod()); 
+        sink(h.getAllow().toArray()[0]);
+        sink(h.getCacheControl()); // $hasTaintFlow
+        sink(h.getConnection().get(0)); // $hasTaintFlow
+        sink(h.getContentDisposition());
+        sink(h.getContentLanguage());
+        sink(h.getContentLength());
+        sink(h.getContentType());
+        sink(h.getDate());
+        sink(h.getETag()); // $hasTaintFlow
+        sink(h.getExpires());
+        sink(h.getFirst("a")); // $hasTaintFlow
+        sink(h.getFirstDate("a")); 
+        sink(h.getFirstZonedDateTime("a")); 
+        sink(h.getHost()); // $hasTaintFlow
+        sink(h.getIfMatch().get(0)); // $hasTaintFlow
+        sink(h.getIfModifiedSince()); 
+        sink(h.getIfNoneMatch().get(0)); // $hasTaintFlow
+        sink(h.getIfUnmodifiedSince()); 
+        sink(h.getLastModified()); 
+        sink(h.getLocation()); // $hasTaintFlow
+        sink(h.getOrEmpty("a").get(0)); // $hasTaintFlow
+        sink(h.getOrigin()); // $hasTaintFlow
+        sink(h.getPragma()); // $hasTaintFlow
+        sink(h.getUpgrade()); // $hasTaintFlow
+        sink(h.getValuesAsList("a").get(0)); // $hasTaintFlow
+        sink(h.getVary().get(0)); // $hasTaintFlow
+    }
 }