From d77d0c9e1071d4bd3af7ab85e9ace2c997531020 Mon Sep 17 00:00:00 2001 From: Tony Torralba Date: Mon, 7 Jun 2021 17:35:03 +0200 Subject: [PATCH 1/9] Added summaries for Spring PropertyValues --- .../code/java/dataflow/ExternalFlow.qll | 1 + .../code/java/frameworks/spring/Spring.qll | 1 + .../java/frameworks/spring/SpringBeans.qll | 41 ++++++ .../frameworks/spring/beans/Test.java | 83 +++++++++++ .../frameworks/spring/beans/options | 1 + .../frameworks/spring/beans/test.expected | 0 .../frameworks/spring/beans/test.ql | 67 +++++++++ .../beans/MutablePropertyValues.java | 135 ++++++++++++++++++ .../springframework/beans/PropertyValue.java | 70 +++++++++ .../springframework/beans/PropertyValues.java | 46 ++++++ 10 files changed, 445 insertions(+) create mode 100644 java/ql/src/semmle/code/java/frameworks/spring/SpringBeans.qll create mode 100644 java/ql/test/library-tests/frameworks/spring/beans/Test.java create mode 100644 java/ql/test/library-tests/frameworks/spring/beans/options create mode 100644 java/ql/test/library-tests/frameworks/spring/beans/test.expected create mode 100644 java/ql/test/library-tests/frameworks/spring/beans/test.ql create mode 100644 java/ql/test/stubs/springframework-5.2.3/org/springframework/beans/MutablePropertyValues.java create mode 100644 java/ql/test/stubs/springframework-5.2.3/org/springframework/beans/PropertyValue.java create mode 100644 java/ql/test/stubs/springframework-5.2.3/org/springframework/beans/PropertyValues.java diff --git a/java/ql/src/semmle/code/java/dataflow/ExternalFlow.qll b/java/ql/src/semmle/code/java/dataflow/ExternalFlow.qll index 8080bd28ab6..506a3456b44 100644 --- a/java/ql/src/semmle/code/java/dataflow/ExternalFlow.qll +++ b/java/ql/src/semmle/code/java/dataflow/ExternalFlow.qll @@ -87,6 +87,7 @@ private module Frameworks { private import semmle.code.java.security.LdapInjection private import semmle.code.java.security.XPath private import semmle.code.java.security.JexlInjection + private import semmle.code.java.frameworks.spring.Spring } private predicate sourceModelCsv(string row) { diff --git a/java/ql/src/semmle/code/java/frameworks/spring/Spring.qll b/java/ql/src/semmle/code/java/frameworks/spring/Spring.qll index 2b09288610e..648b9beb0a0 100644 --- a/java/ql/src/semmle/code/java/frameworks/spring/Spring.qll +++ b/java/ql/src/semmle/code/java/frameworks/spring/Spring.qll @@ -6,6 +6,7 @@ import semmle.code.java.frameworks.spring.SpringAttribute import semmle.code.java.frameworks.spring.SpringAutowire import semmle.code.java.frameworks.spring.SpringBean import semmle.code.java.frameworks.spring.SpringBeanFile +import semmle.code.java.frameworks.spring.SpringBeans import semmle.code.java.frameworks.spring.SpringBeanRefType import semmle.code.java.frameworks.spring.SpringComponentScan import semmle.code.java.frameworks.spring.SpringConstructorArg diff --git a/java/ql/src/semmle/code/java/frameworks/spring/SpringBeans.qll b/java/ql/src/semmle/code/java/frameworks/spring/SpringBeans.qll new file mode 100644 index 00000000000..6c3936ee772 --- /dev/null +++ b/java/ql/src/semmle/code/java/frameworks/spring/SpringBeans.qll @@ -0,0 +1,41 @@ +import java +import semmle.code.java.dataflow.ExternalFlow + +module SpringBeans { + private class FlowSummaries extends SummaryModelCsv { + override predicate row(string row) { + row = + [ + "org.springframework.beans;PropertyValue;false;PropertyValue;(String,Object);;Argument[0];MapKey of Argument[-1];value", + "org.springframework.beans;PropertyValue;false;PropertyValue;(String,Object);;Argument[1];MapValue of Argument[-1];value", + "org.springframework.beans;PropertyValue;false;PropertyValue;(PropertyValue);;Argument[0];Argument[-1];value", + "org.springframework.beans;PropertyValue;false;PropertyValue;(PropertyValue,Object);;MapKey of Argument[0];MapKey of Argument[-1];value", + "org.springframework.beans;PropertyValue;false;getName;;;MapKey of Argument[-1];ReturnValue;value", + "org.springframework.beans;PropertyValue;false;getValue;;;MapValue of Argument[-1];ReturnValue;value", + "org.springframework.beans;PropertyValues;true;getPropertyValue;;;MapValue of Argument[-1];ReturnValue;value", + "org.springframework.beans;PropertyValues;true;getPropertyValues;;;Element of Argument[-1];ArrayElement of ReturnValue;value", + "org.springframework.beans;MutablePropertyValues;true;add;(String,Object);;Argument[0];MapKey of Element of Argument[-1];value", + "org.springframework.beans;MutablePropertyValues;true;add;(String,Object);;Argument[0];MapKey of Element of ReturnValue;value", + "org.springframework.beans;MutablePropertyValues;true;add;(String,Object);;Argument[1];MapValue of Element of Argument[-1];value", + "org.springframework.beans;MutablePropertyValues;true;add;(String,Object);;Argument[1];MapValue of Element of ReturnValue;value", + "org.springframework.beans;MutablePropertyValues;true;addPropertyValue;(PropertyValue);;Argument[0];Element of Argument[-1];value", + "org.springframework.beans;MutablePropertyValues;true;addPropertyValue;(PropertyValue);;Argument[0];Element of ReturnValue;value", + "org.springframework.beans;MutablePropertyValues;true;addPropertyValue;(String,Object);;Argument[0];MapKey of Element of Argument[-1];value", + "org.springframework.beans;MutablePropertyValues;true;addPropertyValue;(String,Object);;Argument[1];MapValue of Element of Argument[-1];value", + "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(Map);;MapKey of Argument[0];MapKey of Element of Argument[-1];value", + "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(Map);;MapKey of Argument[0];MapKey of Element of ReturnValue;value", + "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(Map);;MapValue of Argument[0];MapValue of Element of Argument[-1];value", + "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(Map);;MapValue of Argument[0];MapValue of Element of ReturnValue;value", + "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(PropertyValues);;MapKey of Element of Argument[0];MapKey of Element of Argument[-1];value", + "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(PropertyValues);;MapKey of Element of Argument[0];MapKey of Element of ReturnValue;value", + "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(PropertyValues);;MapValue of Element of Argument[0];MapValue of Element of Argument[-1];value", + "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(PropertyValues);;MapValue of Element of Argument[0];MapValue of Element of ReturnValue;value", + "org.springframework.beans;MutablePropertyValues;true;get;;;MapValue of Element of Argument[-1];ReturnValue;value", + "org.springframework.beans;MutablePropertyValues;true;getPropertyValue;;;Element of Argument[-1];ReturnValue;value", + "org.springframework.beans;MutablePropertyValues;true;getPropertyValueList;;;Element of Argument[-1];Element of ReturnValue;value", + "org.springframework.beans;MutablePropertyValues;true;getPropertyValues;;;Element of Argument[-1];ArrayElement of ReturnValue;value", + "org.springframework.beans;MutablePropertyValues;true;setPropertyValueAt;;;Argument[0];Element of Argument[-1];value" + ] + } + } +} diff --git a/java/ql/test/library-tests/frameworks/spring/beans/Test.java b/java/ql/test/library-tests/frameworks/spring/beans/Test.java new file mode 100644 index 00000000000..0075ea88cf9 --- /dev/null +++ b/java/ql/test/library-tests/frameworks/spring/beans/Test.java @@ -0,0 +1,83 @@ +package generatedtest; + +import org.springframework.beans.PropertyValue; + + +public class Test { + Object getMapKey(Object container) { + return null; + } + + Object getMapValue(Object container) { + return null; + } + + Object newWithMapKey(Object element) { + return null; + } + + Object newWithMapValue(Object element) { + return null; + } + + Object source() { + return null; + } + + void sink(Object o) {} + + public void test() { + // @formatter:off + // "org.springframework.beans;PropertyValue;false;;(String,Object);;Argument[0];MapKey of Argument[-1];value", + { + PropertyValue v = new PropertyValue((String) source(), null); + sink(newWithMapKey(v)); // $hasValueFlow + sink(newWithMapValue(v)); // Safe + } + // "org.springframework.beans;PropertyValue;false;;(String,Object);;Argument[1];MapValue of Argument[-1];value", + { + PropertyValue v = new PropertyValue("", source()); + sink(newWithMapKey(v)); // Safe + sink(newWithMapValue(v)); // $hasValueFlow + } + // "org.springframework.beans;PropertyValue;false;;(PropertyValue);;Argument[0];Argument[-1];value", + { + PropertyValue v1 = new PropertyValue((String) source(), null); + PropertyValue v2 = new PropertyValue(v1); + sink(newWithMapKey(v2)); // $hasValueFlow + sink(newWithMapValue(v2)); // Safe + PropertyValue v3 = new PropertyValue("", source()); + PropertyValue v4 = new PropertyValue(v3); + sink(newWithMapKey(v4)); // Safe + sink(newWithMapValue(v4)); // $hasValueFlow + } + // "org.springframework.beans;PropertyValue;false;;(PropertyValue,Object);;MapKey of Argument[0];MapKey of Argument[-1];value", + // "org.springframework.beans;PropertyValue;false;getName;;;MapKey of Argument[-1];ReturnValue;value", + // "org.springframework.beans;PropertyValue;false;getValue;;;MapValue of Argument[-1];ReturnValue;value", + // "org.springframework.beans;PropertyValues;true;getPropertyValue;;;MapValue of Argument[-1];ReturnValue;value", + // "org.springframework.beans;PropertyValues;true;getPropertyValues;;;Element of Argument[-1];ArrayElement of ReturnValue;value", + // "org.springframework.beans;MutablePropertyValues;true;add;(String,Object);;Argument[0];MapKey of Element of Argument[-1];value", + // "org.springframework.beans;MutablePropertyValues;true;add;(String,Object);;Argument[0];MapKey of Element of ReturnValue;value", + // "org.springframework.beans;MutablePropertyValues;true;add;(String,Object);;Argument[1];MapValue of Element of Argument[-1];value", + // "org.springframework.beans;MutablePropertyValues;true;add;(String,Object);;Argument[1];MapValue of Element of ReturnValue;value", + // "org.springframework.beans;MutablePropertyValues;true;addPropertyValue;(PropertyValue);;Argument[0];Element of Argument[-1];value", + // "org.springframework.beans;MutablePropertyValues;true;addPropertyValue;(PropertyValue);;Argument[0];Element of ReturnValue;value", + // "org.springframework.beans;MutablePropertyValues;true;addPropertyValue;(String,Object);;Argument[0];MapKey of Element of Argument[-1];value", + // "org.springframework.beans;MutablePropertyValues;true;addPropertyValue;(String,Object);;Argument[1];MapValue of Element of Argument[-1];value", + // "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(Map);;MapKey of Argument[0];MapKey of Element of Argument[-1];value", + // "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(Map);;MapKey of Argument[0];MapKey of Element of ReturnValue;value", + // "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(Map);;MapValue of Argument[0];MapValue of Element of Argument[-1];value", + // "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(Map);;MapValue of Argument[0];MapValue of Element of ReturnValue;value", + // "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(PropertyValues);;MapKey of Element of Argument[0];MapKey of Element of Argument[-1];value", + // "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(PropertyValues);;MapKey of Element of Argument[0];MapKey of Element of ReturnValue;value", + // "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(PropertyValues);;MapValue of Element of Argument[0];MapValue of Element of Argument[-1];value", + // "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(PropertyValues);;MapValue of Element of Argument[0];MapValue of Element of ReturnValue;value", + // "org.springframework.beans;MutablePropertyValues;true;get;;;MapValue of Element of Argument[-1];ReturnValue;value", + // "org.springframework.beans;MutablePropertyValues;true;getPropertyValue;;;Element of Argument[-1];ReturnValue;value", + // "org.springframework.beans;MutablePropertyValues;true;getPropertyValueList;;;Element of Argument[-1];Element of ReturnValue;value", + // "org.springframework.beans;MutablePropertyValues;true;getPropertyValues;;;Element of Argument[-1];ArrayElement of ReturnValue;value", + // "org.springframework.beans;MutablePropertyValues;true;setPropertyValueAt;;;Argument[0];Element of Argument[-1];value" + // @formatter:on + + } +} diff --git a/java/ql/test/library-tests/frameworks/spring/beans/options b/java/ql/test/library-tests/frameworks/spring/beans/options new file mode 100644 index 00000000000..31b8e3f6935 --- /dev/null +++ b/java/ql/test/library-tests/frameworks/spring/beans/options @@ -0,0 +1 @@ +//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/springframework-5.2.3 \ No newline at end of file diff --git a/java/ql/test/library-tests/frameworks/spring/beans/test.expected b/java/ql/test/library-tests/frameworks/spring/beans/test.expected new file mode 100644 index 00000000000..e69de29bb2d diff --git a/java/ql/test/library-tests/frameworks/spring/beans/test.ql b/java/ql/test/library-tests/frameworks/spring/beans/test.ql new file mode 100644 index 00000000000..610228239e3 --- /dev/null +++ b/java/ql/test/library-tests/frameworks/spring/beans/test.ql @@ -0,0 +1,67 @@ +import java +import semmle.code.java.dataflow.ExternalFlow +import semmle.code.java.dataflow.TaintTracking +import TestUtilities.InlineExpectationsTest +import semmle.code.java.dataflow.internal.FlowSummaryImpl + +class SummaryModelTest extends SummaryModelCsv { + override predicate row(string row) { + row = + [ + //"package;type;overrides;name;signature;ext;inputspec;outputspec;kind", + "generatedtest;Test;false;getMapKey;;;MapKey of Argument[0];ReturnValue;value", + "generatedtest;Test;false;getMapValue;;;MapValue of Argument[0];ReturnValue;value", + "generatedtest;Test;false;newWithElement;;;Argument[0];Element of ReturnValue;value", + "generatedtest;Test;false;newWithMapKey;;;Argument[0];MapKey of ReturnValue;value", + "generatedtest;Test;false;newWithMapValue;;;Argument[0];MapValue of ReturnValue;value" + ] + } +} + +class ValueFlowConf extends DataFlow::Configuration { + ValueFlowConf() { this = "qltest:valueFlowConf" } + + override predicate isSource(DataFlow::Node n) { + n.asExpr().(MethodAccess).getMethod().hasName("source") + } + + override predicate isSink(DataFlow::Node n) { + n.asExpr().(Argument).getCall().getCallee().hasName("sink") + } +} + +class TaintFlowConf extends TaintTracking::Configuration { + TaintFlowConf() { this = "qltest:taintFlowConf" } + + override predicate isSource(DataFlow::Node n) { + n.asExpr().(MethodAccess).getMethod().hasName("source") + } + + override predicate isSink(DataFlow::Node n) { + n.asExpr().(Argument).getCall().getCallee().hasName("sink") + } +} + +class HasFlowTest extends InlineExpectationsTest { + HasFlowTest() { this = "HasFlowTest" } + + override string getARelevantTag() { result = ["hasValueFlow", "hasTaintFlow"] } + + override predicate hasActualResult(Location location, string element, string tag, string value) { + tag = "hasValueFlow" and + exists(DataFlow::Node src, DataFlow::Node sink, ValueFlowConf conf | conf.hasFlow(src, sink) | + sink.getLocation() = location and + element = sink.toString() and + value = "" + ) + or + tag = "hasTaintFlow" and + exists(DataFlow::Node src, DataFlow::Node sink, TaintFlowConf conf | + conf.hasFlow(src, sink) and not any(ValueFlowConf c).hasFlow(src, sink) + | + sink.getLocation() = location and + element = sink.toString() and + value = "" + ) + } +} diff --git a/java/ql/test/stubs/springframework-5.2.3/org/springframework/beans/MutablePropertyValues.java b/java/ql/test/stubs/springframework-5.2.3/org/springframework/beans/MutablePropertyValues.java new file mode 100644 index 00000000000..fed50425d90 --- /dev/null +++ b/java/ql/test/stubs/springframework-5.2.3/org/springframework/beans/MutablePropertyValues.java @@ -0,0 +1,135 @@ +/* + * Copyright 2002-2020 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ + +package org.springframework.beans; + +import java.io.Serializable; +import java.util.Iterator; +import java.util.List; +import java.util.Map; +import java.util.Spliterator; +import java.util.stream.Stream; +import org.springframework.lang.Nullable; + +public class MutablePropertyValues implements PropertyValues, Serializable { + public MutablePropertyValues() {} + + public MutablePropertyValues(@Nullable PropertyValues original) {} + + public MutablePropertyValues(@Nullable Map original) {} + + public MutablePropertyValues(@Nullable List propertyValueList) {} + + public List getPropertyValueList() { + return null; + } + + public int size() { + return 0; + } + + public MutablePropertyValues addPropertyValues(@Nullable PropertyValues other) { + return null; + } + + public MutablePropertyValues addPropertyValues(@Nullable Map other) { + return null; + } + + public MutablePropertyValues addPropertyValue(PropertyValue pv) { + return null; + } + + public void addPropertyValue(String propertyName, Object propertyValue) {} + + public MutablePropertyValues add(String propertyName, @Nullable Object propertyValue) { + return null; + } + + public void setPropertyValueAt(PropertyValue pv, int i) {} + + public void removePropertyValue(PropertyValue pv) {} + + public void removePropertyValue(String propertyName) {} + + @Override + public Iterator iterator() { + return null; + } + + @Override + public Spliterator spliterator() { + return null; + } + + @Override + public Stream stream() { + return null; + } + + @Override + public PropertyValue[] getPropertyValues() { + return null; + } + + @Override + public PropertyValue getPropertyValue(String propertyName) { + return null; + } + + public Object get(String propertyName) { + return null; + } + + @Override + public PropertyValues changesSince(PropertyValues old) { + return null; + } + + @Override + public boolean contains(String propertyName) { + return false; + } + + @Override + public boolean isEmpty() { + return false; + } + + public void registerProcessedProperty(String propertyName) {} + + public void clearProcessedProperty(String propertyName) {} + + public void setConverted() {} + + public boolean isConverted() { + return false; + } + + @Override + public boolean equals(@Nullable Object other) { + return false; + } + + @Override + public int hashCode() { + return 0; + } + + @Override + public String toString() { + return null; + } + +} diff --git a/java/ql/test/stubs/springframework-5.2.3/org/springframework/beans/PropertyValue.java b/java/ql/test/stubs/springframework-5.2.3/org/springframework/beans/PropertyValue.java new file mode 100644 index 00000000000..2787aa04f17 --- /dev/null +++ b/java/ql/test/stubs/springframework-5.2.3/org/springframework/beans/PropertyValue.java @@ -0,0 +1,70 @@ +/* + * Copyright 2002-2019 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ + +package org.springframework.beans; + +import java.io.Serializable; +import org.springframework.lang.Nullable; + +public class PropertyValue implements Serializable { + public PropertyValue(String name, @Nullable Object value) {} + + public PropertyValue(PropertyValue original) {} + + public PropertyValue(PropertyValue original, @Nullable Object newValue) {} + + public String getName() { + return null; + } + + public Object getValue() { + return null; + } + + public PropertyValue getOriginalPropertyValue() { + return null; + } + + public void setOptional(boolean optional) {} + + public boolean isOptional() { + return false; + } + + public synchronized boolean isConverted() { + return false; + } + + public synchronized void setConvertedValue(@Nullable Object value) {} + + public synchronized Object getConvertedValue() { + return null; + } + + @Override + public boolean equals(@Nullable Object other) { + return false; + } + + @Override + public int hashCode() { + return 0; + } + + @Override + public String toString() { + return null; + } + +} diff --git a/java/ql/test/stubs/springframework-5.2.3/org/springframework/beans/PropertyValues.java b/java/ql/test/stubs/springframework-5.2.3/org/springframework/beans/PropertyValues.java new file mode 100644 index 00000000000..9d5b1f51ec3 --- /dev/null +++ b/java/ql/test/stubs/springframework-5.2.3/org/springframework/beans/PropertyValues.java @@ -0,0 +1,46 @@ +/* + * Copyright 2002-2018 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ + +package org.springframework.beans; + +import java.util.Iterator; +import java.util.Spliterator; +import java.util.stream.Stream; + +public interface PropertyValues extends Iterable { + @Override + default Iterator iterator() { + return null; + } + + @Override + default Spliterator spliterator() { + return null; + } + + default Stream stream() { + return null; + } + + PropertyValue[] getPropertyValues(); + + PropertyValue getPropertyValue(String propertyName); + + PropertyValues changesSince(PropertyValues old); + + boolean contains(String propertyName); + + boolean isEmpty(); + +} From 48b0df4a3efe694f6587510c8811a4dba62438c8 Mon Sep 17 00:00:00 2001 From: Tony Torralba Date: Tue, 8 Jun 2021 10:35:18 +0200 Subject: [PATCH 2/9] Add tests, minor bugfixes --- .../code/java/dataflow/ExternalFlow.qll | 2 +- .../java/frameworks/spring/SpringBeans.qll | 3 +- .../frameworks/spring/beans/Test.java | 268 ++++++++++++++++-- .../frameworks/spring/beans/test.ql | 3 +- 4 files changed, 246 insertions(+), 30 deletions(-) diff --git a/java/ql/src/semmle/code/java/dataflow/ExternalFlow.qll b/java/ql/src/semmle/code/java/dataflow/ExternalFlow.qll index 506a3456b44..7e1a84f99f5 100644 --- a/java/ql/src/semmle/code/java/dataflow/ExternalFlow.qll +++ b/java/ql/src/semmle/code/java/dataflow/ExternalFlow.qll @@ -81,13 +81,13 @@ private module Frameworks { private import semmle.code.java.frameworks.apache.Lang private import semmle.code.java.frameworks.guava.Guava private import semmle.code.java.frameworks.jackson.JacksonSerializability + private import semmle.code.java.frameworks.spring.SpringBeans private import semmle.code.java.security.ResponseSplitting private import semmle.code.java.security.InformationLeak private import semmle.code.java.security.XSS private import semmle.code.java.security.LdapInjection private import semmle.code.java.security.XPath private import semmle.code.java.security.JexlInjection - private import semmle.code.java.frameworks.spring.Spring } private predicate sourceModelCsv(string row) { diff --git a/java/ql/src/semmle/code/java/frameworks/spring/SpringBeans.qll b/java/ql/src/semmle/code/java/frameworks/spring/SpringBeans.qll index 6c3936ee772..ef4b32f5735 100644 --- a/java/ql/src/semmle/code/java/frameworks/spring/SpringBeans.qll +++ b/java/ql/src/semmle/code/java/frameworks/spring/SpringBeans.qll @@ -10,9 +10,10 @@ module SpringBeans { "org.springframework.beans;PropertyValue;false;PropertyValue;(String,Object);;Argument[1];MapValue of Argument[-1];value", "org.springframework.beans;PropertyValue;false;PropertyValue;(PropertyValue);;Argument[0];Argument[-1];value", "org.springframework.beans;PropertyValue;false;PropertyValue;(PropertyValue,Object);;MapKey of Argument[0];MapKey of Argument[-1];value", + "org.springframework.beans;PropertyValue;false;PropertyValue;(PropertyValue,Object);;Argument[1];MapValue of Argument[-1];value", "org.springframework.beans;PropertyValue;false;getName;;;MapKey of Argument[-1];ReturnValue;value", "org.springframework.beans;PropertyValue;false;getValue;;;MapValue of Argument[-1];ReturnValue;value", - "org.springframework.beans;PropertyValues;true;getPropertyValue;;;MapValue of Argument[-1];ReturnValue;value", + "org.springframework.beans;PropertyValues;true;getPropertyValue;;;MapValue of Element of Argument[-1];ReturnValue;value", "org.springframework.beans;PropertyValues;true;getPropertyValues;;;Element of Argument[-1];ArrayElement of ReturnValue;value", "org.springframework.beans;MutablePropertyValues;true;add;(String,Object);;Argument[0];MapKey of Element of Argument[-1];value", "org.springframework.beans;MutablePropertyValues;true;add;(String,Object);;Argument[0];MapKey of Element of ReturnValue;value", diff --git a/java/ql/test/library-tests/frameworks/spring/beans/Test.java b/java/ql/test/library-tests/frameworks/spring/beans/Test.java index 0075ea88cf9..6b33ba707ff 100644 --- a/java/ql/test/library-tests/frameworks/spring/beans/Test.java +++ b/java/ql/test/library-tests/frameworks/spring/beans/Test.java @@ -1,6 +1,11 @@ package generatedtest; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import org.springframework.beans.MutablePropertyValues; import org.springframework.beans.PropertyValue; +import org.springframework.beans.PropertyValues; public class Test { @@ -12,6 +17,14 @@ public class Test { return null; } + Object getElement(Object container) { + return null; + } + + Object getArrayElement(Object container) { + return null; + } + Object newWithMapKey(Object element) { return null; } @@ -20,6 +33,10 @@ public class Test { return null; } + Object newWithElement(Object element) { + return null; + } + Object source() { return null; } @@ -31,53 +48,250 @@ public class Test { // "org.springframework.beans;PropertyValue;false;;(String,Object);;Argument[0];MapKey of Argument[-1];value", { PropertyValue v = new PropertyValue((String) source(), null); - sink(newWithMapKey(v)); // $hasValueFlow - sink(newWithMapValue(v)); // Safe + sink(getMapKey(v)); // $hasValueFlow + sink(getMapValue(v)); // Safe } // "org.springframework.beans;PropertyValue;false;;(String,Object);;Argument[1];MapValue of Argument[-1];value", { PropertyValue v = new PropertyValue("", source()); - sink(newWithMapKey(v)); // Safe - sink(newWithMapValue(v)); // $hasValueFlow + sink(getMapKey(v)); // Safe + sink(getMapValue(v)); // $hasValueFlow } // "org.springframework.beans;PropertyValue;false;;(PropertyValue);;Argument[0];Argument[-1];value", { PropertyValue v1 = new PropertyValue((String) source(), null); PropertyValue v2 = new PropertyValue(v1); - sink(newWithMapKey(v2)); // $hasValueFlow - sink(newWithMapValue(v2)); // Safe - PropertyValue v3 = new PropertyValue("", source()); + sink(getMapKey(v2)); // $hasValueFlow + sink(getMapValue(v2)); // Safe + + PropertyValue v3 = new PropertyValue("safe", source()); PropertyValue v4 = new PropertyValue(v3); - sink(newWithMapKey(v4)); // Safe - sink(newWithMapValue(v4)); // $hasValueFlow + sink(getMapKey(v4)); // Safe + sink(getMapValue(v4)); // $hasValueFlow } // "org.springframework.beans;PropertyValue;false;;(PropertyValue,Object);;MapKey of Argument[0];MapKey of Argument[-1];value", + { + PropertyValue v1 = new PropertyValue((String) source(), source()); + PropertyValue v2 = new PropertyValue(v1, null); + sink(getMapKey(v2)); // $hasValueFlow + sink(getMapValue(v2)); // Safe + } + // "org.springframework.beans;PropertyValue;false;PropertyValue;(PropertyValue,Object);;Argument[1];MapValue of Argument[-1];value", + { + PropertyValue v1 = new PropertyValue("safe", null); + PropertyValue v2 = new PropertyValue(v1, source()); + sink(getMapKey(v2)); // Safe + sink(getMapValue(v2)); // $hasValueFlow + } // "org.springframework.beans;PropertyValue;false;getName;;;MapKey of Argument[-1];ReturnValue;value", + { + PropertyValue v = new PropertyValue((String) source(), null); + sink(v.getName()); // $hasValueFlow + sink(v.getValue()); // Safe + } // "org.springframework.beans;PropertyValue;false;getValue;;;MapValue of Argument[-1];ReturnValue;value", - // "org.springframework.beans;PropertyValues;true;getPropertyValue;;;MapValue of Argument[-1];ReturnValue;value", + { + PropertyValue v = new PropertyValue("safe", source()); + sink(v.getName()); // Safe + sink(v.getValue()); // $hasValueFlow + } + // "org.springframework.beans;PropertyValues;true;getPropertyValue;;;MapValue of Element of Argument[-1];ReturnValue;value", + { + PropertyValues pv = (PropertyValues) newWithElement(newWithMapValue(source())); + sink(pv.getPropertyValue("safe")); // $hasValueFlow + } // "org.springframework.beans;PropertyValues;true;getPropertyValues;;;Element of Argument[-1];ArrayElement of ReturnValue;value", + { + PropertyValues pv = (PropertyValues) newWithElement(newWithMapValue(source())); + PropertyValue[] vs = pv.getPropertyValues(); + sink(getMapKey(getArrayElement(vs))); // Safe + sink(getMapValue(getArrayElement(vs))); // $hasValueFlow + } // "org.springframework.beans;MutablePropertyValues;true;add;(String,Object);;Argument[0];MapKey of Element of Argument[-1];value", + { + MutablePropertyValues pv = new MutablePropertyValues(); + pv.add((String) source(), null); + sink(getMapKey(getElement(pv))); // $hasValueFlow + sink(getMapValue(getElement(pv))); // Safe + } // "org.springframework.beans;MutablePropertyValues;true;add;(String,Object);;Argument[0];MapKey of Element of ReturnValue;value", + { + MutablePropertyValues pv = new MutablePropertyValues(); + sink(getMapKey(getElement(pv.add((String) source(), null)))); // $hasValueFlow + sink(getMapValue(getElement(pv.add((String) source(), null)))); // Safe + } // "org.springframework.beans;MutablePropertyValues;true;add;(String,Object);;Argument[1];MapValue of Element of Argument[-1];value", + { + MutablePropertyValues pv = new MutablePropertyValues(); + pv.add("safe", source()); + sink(getMapKey(getElement(pv))); // Safe + sink(getMapValue(getElement(pv))); // $hasValueFlow + } // "org.springframework.beans;MutablePropertyValues;true;add;(String,Object);;Argument[1];MapValue of Element of ReturnValue;value", + { + MutablePropertyValues pv = new MutablePropertyValues(); + sink(getMapKey(getElement(pv.add("safe", source())))); // Safe + sink(getMapValue(getElement(pv.add("safe", source())))); // $hasValueFlow + } // "org.springframework.beans;MutablePropertyValues;true;addPropertyValue;(PropertyValue);;Argument[0];Element of Argument[-1];value", - // "org.springframework.beans;MutablePropertyValues;true;addPropertyValue;(PropertyValue);;Argument[0];Element of ReturnValue;value", - // "org.springframework.beans;MutablePropertyValues;true;addPropertyValue;(String,Object);;Argument[0];MapKey of Element of Argument[-1];value", - // "org.springframework.beans;MutablePropertyValues;true;addPropertyValue;(String,Object);;Argument[1];MapValue of Element of Argument[-1];value", - // "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(Map);;MapKey of Argument[0];MapKey of Element of Argument[-1];value", - // "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(Map);;MapKey of Argument[0];MapKey of Element of ReturnValue;value", - // "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(Map);;MapValue of Argument[0];MapValue of Element of Argument[-1];value", - // "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(Map);;MapValue of Argument[0];MapValue of Element of ReturnValue;value", - // "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(PropertyValues);;MapKey of Element of Argument[0];MapKey of Element of Argument[-1];value", - // "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(PropertyValues);;MapKey of Element of Argument[0];MapKey of Element of ReturnValue;value", - // "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(PropertyValues);;MapValue of Element of Argument[0];MapValue of Element of Argument[-1];value", - // "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(PropertyValues);;MapValue of Element of Argument[0];MapValue of Element of ReturnValue;value", - // "org.springframework.beans;MutablePropertyValues;true;get;;;MapValue of Element of Argument[-1];ReturnValue;value", - // "org.springframework.beans;MutablePropertyValues;true;getPropertyValue;;;Element of Argument[-1];ReturnValue;value", - // "org.springframework.beans;MutablePropertyValues;true;getPropertyValueList;;;Element of Argument[-1];Element of ReturnValue;value", - // "org.springframework.beans;MutablePropertyValues;true;getPropertyValues;;;Element of Argument[-1];ArrayElement of ReturnValue;value", - // "org.springframework.beans;MutablePropertyValues;true;setPropertyValueAt;;;Argument[0];Element of Argument[-1];value" - // @formatter:on + { + MutablePropertyValues pv1 = new MutablePropertyValues(); + PropertyValue v1 = (PropertyValue) newWithMapKey(source()); + pv1.addPropertyValue(v1); + sink(getMapKey(getElement(pv1))); // $hasValueFlow + sink(getMapValue(getElement(pv1))); // Safe + MutablePropertyValues pv2 = new MutablePropertyValues(); + PropertyValue v2 = (PropertyValue) newWithMapValue(source()); + pv2.addPropertyValue(v2); + sink(getMapKey(getElement(pv2))); // Safe + sink(getMapValue(getElement(pv2))); // $hasValueFlow + } + // "org.springframework.beans;MutablePropertyValues;true;addPropertyValue;(PropertyValue);;Argument[0];Element of ReturnValue;value", + { + MutablePropertyValues pv1 = new MutablePropertyValues(); + PropertyValue v1 = (PropertyValue) newWithMapKey(source()); + PropertyValues pv2 = pv1.addPropertyValue(v1); + sink(getMapKey(getElement(pv2))); // $hasValueFlow + sink(getMapValue(getElement(pv2))); // Safe + + MutablePropertyValues pv3 = new MutablePropertyValues(); + PropertyValue v2 = (PropertyValue) newWithMapValue(source()); + PropertyValues pv4 = pv3.addPropertyValue(v2); + sink(getMapKey(getElement(pv4))); // Safe + sink(getMapValue(getElement(pv4))); // $hasValueFlow + } + // "org.springframework.beans;MutablePropertyValues;true;addPropertyValue;(String,Object);;Argument[0];MapKey of Element of Argument[-1];value", + { + MutablePropertyValues pv = new MutablePropertyValues(); + pv.addPropertyValue((String)source(), null); + sink(getMapKey(getElement(pv))); // $hasValueFlow + sink(getMapValue(getElement(pv))); // Safe + } + // "org.springframework.beans;MutablePropertyValues;true;addPropertyValue;(String,Object);;Argument[1];MapValue of Element of Argument[-1];value", + { + MutablePropertyValues pv = new MutablePropertyValues(); + pv.addPropertyValue("safe", source()); + sink(getMapKey(getElement(pv))); // Safe + sink(getMapValue(getElement(pv))); // $hasValueFlow + } + // "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(Map);;MapKey of Argument[0];MapKey of Element of Argument[-1];value", + { + MutablePropertyValues pv = new MutablePropertyValues(); + Map values = (Map) newWithMapKey(source()); + pv.addPropertyValues(values); + sink(getMapKey(getElement(pv))); // $hasValueFlow + sink(getMapValue(getElement(pv))); // Safe + } + // "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(Map);;MapKey of Argument[0];MapKey of Element of ReturnValue;value", + { + MutablePropertyValues pv = new MutablePropertyValues(); + Map values = (Map) newWithMapKey(source()); + PropertyValues pv2 = pv.addPropertyValues(values); + sink(getMapKey(getElement(pv2))); // $hasValueFlow + sink(getMapValue(getElement(pv2))); // Safe + } + // "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(Map);;MapValue of Argument[0];MapValue of Element of Argument[-1];value", + { + MutablePropertyValues pv = new MutablePropertyValues(); + Map values = (Map) newWithMapValue(source()); + pv.addPropertyValues(values); + sink(getMapKey(getElement(pv))); // Safe + sink(getMapValue(getElement(pv))); // $hasValueFlow + } + // "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(Map);;MapValue of Argument[0];MapValue of Element of ReturnValue;value", + { + MutablePropertyValues pv = new MutablePropertyValues(); + Map values = (Map) newWithMapValue(source()); + PropertyValues pv2 = pv.addPropertyValues(values); + sink(getMapKey(getElement(pv2))); // Safe + sink(getMapValue(getElement(pv2))); // $hasValueFlow + } + // "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(PropertyValues);;MapKey of Element of Argument[0];MapKey of Element of Argument[-1];value", + { + MutablePropertyValues pv = new MutablePropertyValues(); + PropertyValues values = (PropertyValues) newWithElement(newWithMapKey(source())); + pv.addPropertyValues(values); + sink(getMapKey(getElement(pv))); // $hasValueFlow + sink(getMapValue(getElement(pv))); // Safe + } + // "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(PropertyValues);;MapKey of Element of Argument[0];MapKey of Element of ReturnValue;value", + { + MutablePropertyValues pv = new MutablePropertyValues(); + PropertyValues values = (PropertyValues) newWithElement(newWithMapKey(source())); + PropertyValues pv2 = pv.addPropertyValues(values); + sink(getMapKey(getElement(pv2))); // $hasValueFlow + sink(getMapValue(getElement(pv2))); // Safe + } + // "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(PropertyValues);;MapValue of Element of Argument[0];MapValue of Element of Argument[-1];value", + { + MutablePropertyValues pv = new MutablePropertyValues(); + PropertyValues values = (PropertyValues) newWithElement(newWithMapValue(source())); + pv.addPropertyValues(values); + sink(getMapKey(getElement(pv))); // Safe + sink(getMapValue(getElement(pv))); // $hasValueFlow + } + // "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(PropertyValues);;MapValue of Element of Argument[0];MapValue of Element of ReturnValue;value", + { + MutablePropertyValues pv = new MutablePropertyValues(); + PropertyValues values = (PropertyValues) newWithElement(newWithMapValue(source())); + PropertyValues pv2 = pv.addPropertyValues(values); + sink(getMapKey(getElement(pv2))); // Safe + sink(getMapValue(getElement(pv2))); // $hasValueFlow + } + // "org.springframework.beans;MutablePropertyValues;true;get;;;MapValue of Element of Argument[-1];ReturnValue;value", + { + MutablePropertyValues pv = (MutablePropertyValues) newWithElement(newWithMapValue(source())); + sink(pv.get("something")); // $hasValueFlow + } + // "org.springframework.beans;MutablePropertyValues;true;getPropertyValue;;;Element of Argument[-1];ReturnValue;value", + { + MutablePropertyValues pv1 = (MutablePropertyValues) newWithElement(newWithMapKey(source())); + sink(pv1.getPropertyValue("something").getName()); // $hasValueFlow + sink(pv1.getPropertyValue("something").getValue()); // Safe + + MutablePropertyValues pv2 = (MutablePropertyValues) newWithElement(newWithMapValue(source())); + sink(pv2.getPropertyValue("something").getName()); // Safe + sink(pv2.getPropertyValue("something").getValue()); // $hasValueFlow + } + // "org.springframework.beans;MutablePropertyValues;true;getPropertyValueList;;;Element of Argument[-1];Element of ReturnValue;value", + { + MutablePropertyValues pv1 = (MutablePropertyValues) newWithElement(newWithMapKey(source())); + List pvl1 = pv1.getPropertyValueList(); + sink(getMapKey(getElement(pvl1))); // $hasValueFlow + sink(getMapValue(getElement(pvl1))); // Safe + + MutablePropertyValues pv2 = (MutablePropertyValues) newWithElement(newWithMapValue(source())); + List pvl2 = pv2.getPropertyValueList(); + sink(getMapKey(getElement(pvl2))); // Safe + sink(getMapValue(getElement(pvl2))); // $hasValueFlow + } + // "org.springframework.beans;MutablePropertyValues;true;getPropertyValues;;;Element of Argument[-1];ArrayElement of ReturnValue;value", + { + MutablePropertyValues pv1 = (MutablePropertyValues) newWithElement(newWithMapKey(source())); + PropertyValue[] pvl1 = pv1.getPropertyValues(); + sink(getMapKey(getArrayElement(pvl1))); // $hasValueFlow + sink(getMapValue(getArrayElement(pvl1))); // Safe + + MutablePropertyValues pv2 = (MutablePropertyValues) newWithElement(newWithMapValue(source())); + PropertyValue[] pvl2 = pv2.getPropertyValues(); + sink(getMapKey(getArrayElement(pvl2))); // Safe + sink(getMapValue(getArrayElement(pvl2))); // $hasValueFlow + } + // "org.springframework.beans;MutablePropertyValues;true;setPropertyValueAt;;;Argument[0];Element of Argument[-1];value" + { + MutablePropertyValues pv1 = new MutablePropertyValues(); + PropertyValue v1 = (PropertyValue) newWithMapKey(source()); + pv1.setPropertyValueAt(v1, 0); + sink(getMapKey(getElement(pv1))); // $hasValueFlow + sink(getMapValue(getElement(pv1))); // Safe + + MutablePropertyValues pv2 = new MutablePropertyValues(); + PropertyValue v2 = (PropertyValue) newWithMapValue(source()); + pv2.setPropertyValueAt(v2, 0); + sink(getMapKey(getElement(pv2))); // Safe + sink(getMapValue(getElement(pv2))); // $hasValueFlow + } + // @formatter:on } } diff --git a/java/ql/test/library-tests/frameworks/spring/beans/test.ql b/java/ql/test/library-tests/frameworks/spring/beans/test.ql index 610228239e3..05800dbc1f2 100644 --- a/java/ql/test/library-tests/frameworks/spring/beans/test.ql +++ b/java/ql/test/library-tests/frameworks/spring/beans/test.ql @@ -2,7 +2,6 @@ import java import semmle.code.java.dataflow.ExternalFlow import semmle.code.java.dataflow.TaintTracking import TestUtilities.InlineExpectationsTest -import semmle.code.java.dataflow.internal.FlowSummaryImpl class SummaryModelTest extends SummaryModelCsv { override predicate row(string row) { @@ -11,6 +10,8 @@ class SummaryModelTest extends SummaryModelCsv { //"package;type;overrides;name;signature;ext;inputspec;outputspec;kind", "generatedtest;Test;false;getMapKey;;;MapKey of Argument[0];ReturnValue;value", "generatedtest;Test;false;getMapValue;;;MapValue of Argument[0];ReturnValue;value", + "generatedtest;Test;false;getElement;;;Element of Argument[0];ReturnValue;value", + "generatedtest;Test;false;getArrayElement;;;ArrayElement of Argument[0];ReturnValue;value", "generatedtest;Test;false;newWithElement;;;Argument[0];Element of ReturnValue;value", "generatedtest;Test;false;newWithMapKey;;;Argument[0];MapKey of ReturnValue;value", "generatedtest;Test;false;newWithMapValue;;;Argument[0];MapValue of ReturnValue;value" From 9024788a92e0b13739e2104d93b7610fa6ccdcdb Mon Sep 17 00:00:00 2001 From: Tony Torralba Date: Tue, 8 Jun 2021 10:42:07 +0200 Subject: [PATCH 3/9] Add change note --- java/change-notes/2021-06-08-spring-propertyvalues.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 java/change-notes/2021-06-08-spring-propertyvalues.md diff --git a/java/change-notes/2021-06-08-spring-propertyvalues.md b/java/change-notes/2021-06-08-spring-propertyvalues.md new file mode 100644 index 00000000000..3e7ae540109 --- /dev/null +++ b/java/change-notes/2021-06-08-spring-propertyvalues.md @@ -0,0 +1,2 @@ +lgtm,codescanning +* Added additional taint steps modeling the Spring classes `PropertyValue`, `PropertyValues` and `MutablePropertyValues`. (`org.springframework.beans.*`). From afab13e7ee00a5bbc4ce99ee4952a026aec8e440 Mon Sep 17 00:00:00 2001 From: Tony Torralba Date: Tue, 8 Jun 2021 11:09:59 +0200 Subject: [PATCH 4/9] Add missing QLDoc --- .../src/semmle/code/java/frameworks/spring/SpringBeans.qll | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/java/ql/src/semmle/code/java/frameworks/spring/SpringBeans.qll b/java/ql/src/semmle/code/java/frameworks/spring/SpringBeans.qll index ef4b32f5735..0c9dbbafaf5 100644 --- a/java/ql/src/semmle/code/java/frameworks/spring/SpringBeans.qll +++ b/java/ql/src/semmle/code/java/frameworks/spring/SpringBeans.qll @@ -1,3 +1,8 @@ +/** + * Provides classes for working with Spring classes and interfaces from + * `org.springframework.beans`. + */ + import java import semmle.code.java.dataflow.ExternalFlow From 498c2250c7e78f01bc01cd86c9b3bcf70c4483c3 Mon Sep 17 00:00:00 2001 From: Tony Torralba Date: Tue, 8 Jun 2021 11:25:53 +0200 Subject: [PATCH 5/9] Add missing QLDoc --- .../src/semmle/code/java/frameworks/spring/SpringBeans.qll | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/java/ql/src/semmle/code/java/frameworks/spring/SpringBeans.qll b/java/ql/src/semmle/code/java/frameworks/spring/SpringBeans.qll index 0c9dbbafaf5..1e4811d58b4 100644 --- a/java/ql/src/semmle/code/java/frameworks/spring/SpringBeans.qll +++ b/java/ql/src/semmle/code/java/frameworks/spring/SpringBeans.qll @@ -1,11 +1,14 @@ /** - * Provides classes for working with Spring classes and interfaces from + * Provides classes and predicates for working with Spring classes and interfaces from * `org.springframework.beans`. */ import java import semmle.code.java.dataflow.ExternalFlow +/** + * Provides models for the `org.springframework.beans` package. + */ module SpringBeans { private class FlowSummaries extends SummaryModelCsv { override predicate row(string row) { From 393b95cbbe2e77c3499da1c28d45b9ca3b639704 Mon Sep 17 00:00:00 2001 From: Tony Torralba Date: Mon, 28 Jun 2021 17:01:34 +0200 Subject: [PATCH 6/9] Remove 'magic' from tests --- .../java/frameworks/spring/SpringBeans.qll | 72 ++++++++------- .../frameworks/spring/beans/Test.java | 88 ++++++++++--------- .../frameworks/spring/beans/test.ql | 16 ---- 3 files changed, 82 insertions(+), 94 deletions(-) diff --git a/java/ql/src/semmle/code/java/frameworks/spring/SpringBeans.qll b/java/ql/src/semmle/code/java/frameworks/spring/SpringBeans.qll index 1e4811d58b4..74c56025fa7 100644 --- a/java/ql/src/semmle/code/java/frameworks/spring/SpringBeans.qll +++ b/java/ql/src/semmle/code/java/frameworks/spring/SpringBeans.qll @@ -9,42 +9,40 @@ import semmle.code.java.dataflow.ExternalFlow /** * Provides models for the `org.springframework.beans` package. */ -module SpringBeans { - private class FlowSummaries extends SummaryModelCsv { - override predicate row(string row) { - row = - [ - "org.springframework.beans;PropertyValue;false;PropertyValue;(String,Object);;Argument[0];MapKey of Argument[-1];value", - "org.springframework.beans;PropertyValue;false;PropertyValue;(String,Object);;Argument[1];MapValue of Argument[-1];value", - "org.springframework.beans;PropertyValue;false;PropertyValue;(PropertyValue);;Argument[0];Argument[-1];value", - "org.springframework.beans;PropertyValue;false;PropertyValue;(PropertyValue,Object);;MapKey of Argument[0];MapKey of Argument[-1];value", - "org.springframework.beans;PropertyValue;false;PropertyValue;(PropertyValue,Object);;Argument[1];MapValue of Argument[-1];value", - "org.springframework.beans;PropertyValue;false;getName;;;MapKey of Argument[-1];ReturnValue;value", - "org.springframework.beans;PropertyValue;false;getValue;;;MapValue of Argument[-1];ReturnValue;value", - "org.springframework.beans;PropertyValues;true;getPropertyValue;;;MapValue of Element of Argument[-1];ReturnValue;value", - "org.springframework.beans;PropertyValues;true;getPropertyValues;;;Element of Argument[-1];ArrayElement of ReturnValue;value", - "org.springframework.beans;MutablePropertyValues;true;add;(String,Object);;Argument[0];MapKey of Element of Argument[-1];value", - "org.springframework.beans;MutablePropertyValues;true;add;(String,Object);;Argument[0];MapKey of Element of ReturnValue;value", - "org.springframework.beans;MutablePropertyValues;true;add;(String,Object);;Argument[1];MapValue of Element of Argument[-1];value", - "org.springframework.beans;MutablePropertyValues;true;add;(String,Object);;Argument[1];MapValue of Element of ReturnValue;value", - "org.springframework.beans;MutablePropertyValues;true;addPropertyValue;(PropertyValue);;Argument[0];Element of Argument[-1];value", - "org.springframework.beans;MutablePropertyValues;true;addPropertyValue;(PropertyValue);;Argument[0];Element of ReturnValue;value", - "org.springframework.beans;MutablePropertyValues;true;addPropertyValue;(String,Object);;Argument[0];MapKey of Element of Argument[-1];value", - "org.springframework.beans;MutablePropertyValues;true;addPropertyValue;(String,Object);;Argument[1];MapValue of Element of Argument[-1];value", - "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(Map);;MapKey of Argument[0];MapKey of Element of Argument[-1];value", - "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(Map);;MapKey of Argument[0];MapKey of Element of ReturnValue;value", - "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(Map);;MapValue of Argument[0];MapValue of Element of Argument[-1];value", - "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(Map);;MapValue of Argument[0];MapValue of Element of ReturnValue;value", - "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(PropertyValues);;MapKey of Element of Argument[0];MapKey of Element of Argument[-1];value", - "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(PropertyValues);;MapKey of Element of Argument[0];MapKey of Element of ReturnValue;value", - "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(PropertyValues);;MapValue of Element of Argument[0];MapValue of Element of Argument[-1];value", - "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(PropertyValues);;MapValue of Element of Argument[0];MapValue of Element of ReturnValue;value", - "org.springframework.beans;MutablePropertyValues;true;get;;;MapValue of Element of Argument[-1];ReturnValue;value", - "org.springframework.beans;MutablePropertyValues;true;getPropertyValue;;;Element of Argument[-1];ReturnValue;value", - "org.springframework.beans;MutablePropertyValues;true;getPropertyValueList;;;Element of Argument[-1];Element of ReturnValue;value", - "org.springframework.beans;MutablePropertyValues;true;getPropertyValues;;;Element of Argument[-1];ArrayElement of ReturnValue;value", - "org.springframework.beans;MutablePropertyValues;true;setPropertyValueAt;;;Argument[0];Element of Argument[-1];value" - ] - } +private class FlowSummaries extends SummaryModelCsv { + override predicate row(string row) { + row = + [ + "org.springframework.beans;PropertyValue;false;PropertyValue;(String,Object);;Argument[0];MapKey of Argument[-1];value", + "org.springframework.beans;PropertyValue;false;PropertyValue;(String,Object);;Argument[1];MapValue of Argument[-1];value", + "org.springframework.beans;PropertyValue;false;PropertyValue;(PropertyValue);;Argument[0];Argument[-1];value", + "org.springframework.beans;PropertyValue;false;PropertyValue;(PropertyValue,Object);;MapKey of Argument[0];MapKey of Argument[-1];value", + "org.springframework.beans;PropertyValue;false;PropertyValue;(PropertyValue,Object);;Argument[1];MapValue of Argument[-1];value", + "org.springframework.beans;PropertyValue;false;getName;;;MapKey of Argument[-1];ReturnValue;value", + "org.springframework.beans;PropertyValue;false;getValue;;;MapValue of Argument[-1];ReturnValue;value", + "org.springframework.beans;PropertyValues;true;getPropertyValue;;;MapValue of Element of Argument[-1];ReturnValue;value", + "org.springframework.beans;PropertyValues;true;getPropertyValues;;;Element of Argument[-1];ArrayElement of ReturnValue;value", + "org.springframework.beans;MutablePropertyValues;true;add;(String,Object);;Argument[0];MapKey of Element of Argument[-1];value", + "org.springframework.beans;MutablePropertyValues;true;add;(String,Object);;Argument[0];MapKey of Element of ReturnValue;value", + "org.springframework.beans;MutablePropertyValues;true;add;(String,Object);;Argument[1];MapValue of Element of Argument[-1];value", + "org.springframework.beans;MutablePropertyValues;true;add;(String,Object);;Argument[1];MapValue of Element of ReturnValue;value", + "org.springframework.beans;MutablePropertyValues;true;addPropertyValue;(PropertyValue);;Argument[0];Element of Argument[-1];value", + "org.springframework.beans;MutablePropertyValues;true;addPropertyValue;(PropertyValue);;Argument[0];Element of ReturnValue;value", + "org.springframework.beans;MutablePropertyValues;true;addPropertyValue;(String,Object);;Argument[0];MapKey of Element of Argument[-1];value", + "org.springframework.beans;MutablePropertyValues;true;addPropertyValue;(String,Object);;Argument[1];MapValue of Element of Argument[-1];value", + "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(Map);;MapKey of Argument[0];MapKey of Element of Argument[-1];value", + "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(Map);;MapKey of Argument[0];MapKey of Element of ReturnValue;value", + "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(Map);;MapValue of Argument[0];MapValue of Element of Argument[-1];value", + "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(Map);;MapValue of Argument[0];MapValue of Element of ReturnValue;value", + "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(PropertyValues);;MapKey of Element of Argument[0];MapKey of Element of Argument[-1];value", + "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(PropertyValues);;MapKey of Element of Argument[0];MapKey of Element of ReturnValue;value", + "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(PropertyValues);;MapValue of Element of Argument[0];MapValue of Element of Argument[-1];value", + "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(PropertyValues);;MapValue of Element of Argument[0];MapValue of Element of ReturnValue;value", + "org.springframework.beans;MutablePropertyValues;true;get;;;MapValue of Element of Argument[-1];ReturnValue;value", + "org.springframework.beans;MutablePropertyValues;true;getPropertyValue;;;Element of Argument[-1];ReturnValue;value", + "org.springframework.beans;MutablePropertyValues;true;getPropertyValueList;;;Element of Argument[-1];Element of ReturnValue;value", + "org.springframework.beans;MutablePropertyValues;true;getPropertyValues;;;Element of Argument[-1];ArrayElement of ReturnValue;value", + "org.springframework.beans;MutablePropertyValues;true;setPropertyValueAt;;;Argument[0];Element of Argument[-1];value" + ] } } diff --git a/java/ql/test/library-tests/frameworks/spring/beans/Test.java b/java/ql/test/library-tests/frameworks/spring/beans/Test.java index 6b33ba707ff..7132f10850c 100644 --- a/java/ql/test/library-tests/frameworks/spring/beans/Test.java +++ b/java/ql/test/library-tests/frameworks/spring/beans/Test.java @@ -9,35 +9,37 @@ import org.springframework.beans.PropertyValues; public class Test { - Object getMapKey(Object container) { - return null; + Object getMapKey(PropertyValue container) { + return container.getName(); } - Object getMapValue(Object container) { - return null; + Object getMapValue(PropertyValue container) { + return container.getValue(); } - Object getElement(Object container) { - return null; + PropertyValue getElement(Iterable container) { + return container.iterator().next(); } - Object getArrayElement(Object container) { - return null; + PropertyValue getArrayElement(PropertyValue[] container) { + return container[0]; } - Object newWithMapKey(Object element) { - return null; + PropertyValue newWithMapKey(String element) { + return new PropertyValue(element, null); } - Object newWithMapValue(Object element) { - return null; + PropertyValue newWithMapValue(String element) { + return new PropertyValue("", element); } - Object newWithElement(Object element) { - return null; + MutablePropertyValues newWithElement(PropertyValue element) { + MutablePropertyValues pv = new MutablePropertyValues(); + pv.addPropertyValue(element); + return pv; } - Object source() { + String source() { return null; } @@ -97,12 +99,12 @@ public class Test { } // "org.springframework.beans;PropertyValues;true;getPropertyValue;;;MapValue of Element of Argument[-1];ReturnValue;value", { - PropertyValues pv = (PropertyValues) newWithElement(newWithMapValue(source())); - sink(pv.getPropertyValue("safe")); // $hasValueFlow + PropertyValues pv = newWithElement(newWithMapValue(source())); + sink(pv.getPropertyValue("safe").getValue()); // $hasValueFlow } // "org.springframework.beans;PropertyValues;true;getPropertyValues;;;Element of Argument[-1];ArrayElement of ReturnValue;value", { - PropertyValues pv = (PropertyValues) newWithElement(newWithMapValue(source())); + PropertyValues pv = newWithElement(newWithMapValue(source())); PropertyValue[] vs = pv.getPropertyValues(); sink(getMapKey(getArrayElement(vs))); // Safe sink(getMapValue(getArrayElement(vs))); // $hasValueFlow @@ -117,8 +119,8 @@ public class Test { // "org.springframework.beans;MutablePropertyValues;true;add;(String,Object);;Argument[0];MapKey of Element of ReturnValue;value", { MutablePropertyValues pv = new MutablePropertyValues(); - sink(getMapKey(getElement(pv.add((String) source(), null)))); // $hasValueFlow - sink(getMapValue(getElement(pv.add((String) source(), null)))); // Safe + sink(getMapKey(getElement(pv.add(source(), null)))); // $hasValueFlow + sink(getMapValue(getElement(pv.add(source(), null)))); // Safe } // "org.springframework.beans;MutablePropertyValues;true;add;(String,Object);;Argument[1];MapValue of Element of Argument[-1];value", { @@ -136,13 +138,13 @@ public class Test { // "org.springframework.beans;MutablePropertyValues;true;addPropertyValue;(PropertyValue);;Argument[0];Element of Argument[-1];value", { MutablePropertyValues pv1 = new MutablePropertyValues(); - PropertyValue v1 = (PropertyValue) newWithMapKey(source()); + PropertyValue v1 = newWithMapKey(source()); pv1.addPropertyValue(v1); sink(getMapKey(getElement(pv1))); // $hasValueFlow sink(getMapValue(getElement(pv1))); // Safe MutablePropertyValues pv2 = new MutablePropertyValues(); - PropertyValue v2 = (PropertyValue) newWithMapValue(source()); + PropertyValue v2 = newWithMapValue(source()); pv2.addPropertyValue(v2); sink(getMapKey(getElement(pv2))); // Safe sink(getMapValue(getElement(pv2))); // $hasValueFlow @@ -150,13 +152,13 @@ public class Test { // "org.springframework.beans;MutablePropertyValues;true;addPropertyValue;(PropertyValue);;Argument[0];Element of ReturnValue;value", { MutablePropertyValues pv1 = new MutablePropertyValues(); - PropertyValue v1 = (PropertyValue) newWithMapKey(source()); + PropertyValue v1 = newWithMapKey(source()); PropertyValues pv2 = pv1.addPropertyValue(v1); sink(getMapKey(getElement(pv2))); // $hasValueFlow sink(getMapValue(getElement(pv2))); // Safe MutablePropertyValues pv3 = new MutablePropertyValues(); - PropertyValue v2 = (PropertyValue) newWithMapValue(source()); + PropertyValue v2 = newWithMapValue(source()); PropertyValues pv4 = pv3.addPropertyValue(v2); sink(getMapKey(getElement(pv4))); // Safe sink(getMapValue(getElement(pv4))); // $hasValueFlow @@ -178,7 +180,8 @@ public class Test { // "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(Map);;MapKey of Argument[0];MapKey of Element of Argument[-1];value", { MutablePropertyValues pv = new MutablePropertyValues(); - Map values = (Map) newWithMapKey(source()); + Map values = new HashMap(); + values.put(source(), null); pv.addPropertyValues(values); sink(getMapKey(getElement(pv))); // $hasValueFlow sink(getMapValue(getElement(pv))); // Safe @@ -186,7 +189,8 @@ public class Test { // "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(Map);;MapKey of Argument[0];MapKey of Element of ReturnValue;value", { MutablePropertyValues pv = new MutablePropertyValues(); - Map values = (Map) newWithMapKey(source()); + Map values = new HashMap(); + values.put(source(), null); PropertyValues pv2 = pv.addPropertyValues(values); sink(getMapKey(getElement(pv2))); // $hasValueFlow sink(getMapValue(getElement(pv2))); // Safe @@ -194,7 +198,8 @@ public class Test { // "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(Map);;MapValue of Argument[0];MapValue of Element of Argument[-1];value", { MutablePropertyValues pv = new MutablePropertyValues(); - Map values = (Map) newWithMapValue(source()); + Map values = new HashMap(); + values.put("", source()); pv.addPropertyValues(values); sink(getMapKey(getElement(pv))); // Safe sink(getMapValue(getElement(pv))); // $hasValueFlow @@ -202,7 +207,8 @@ public class Test { // "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(Map);;MapValue of Argument[0];MapValue of Element of ReturnValue;value", { MutablePropertyValues pv = new MutablePropertyValues(); - Map values = (Map) newWithMapValue(source()); + Map values = new HashMap(); + values.put("", source()); PropertyValues pv2 = pv.addPropertyValues(values); sink(getMapKey(getElement(pv2))); // Safe sink(getMapValue(getElement(pv2))); // $hasValueFlow @@ -210,7 +216,7 @@ public class Test { // "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(PropertyValues);;MapKey of Element of Argument[0];MapKey of Element of Argument[-1];value", { MutablePropertyValues pv = new MutablePropertyValues(); - PropertyValues values = (PropertyValues) newWithElement(newWithMapKey(source())); + PropertyValues values = newWithElement(newWithMapKey(source())); pv.addPropertyValues(values); sink(getMapKey(getElement(pv))); // $hasValueFlow sink(getMapValue(getElement(pv))); // Safe @@ -218,7 +224,7 @@ public class Test { // "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(PropertyValues);;MapKey of Element of Argument[0];MapKey of Element of ReturnValue;value", { MutablePropertyValues pv = new MutablePropertyValues(); - PropertyValues values = (PropertyValues) newWithElement(newWithMapKey(source())); + PropertyValues values = newWithElement(newWithMapKey(source())); PropertyValues pv2 = pv.addPropertyValues(values); sink(getMapKey(getElement(pv2))); // $hasValueFlow sink(getMapValue(getElement(pv2))); // Safe @@ -226,7 +232,7 @@ public class Test { // "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(PropertyValues);;MapValue of Element of Argument[0];MapValue of Element of Argument[-1];value", { MutablePropertyValues pv = new MutablePropertyValues(); - PropertyValues values = (PropertyValues) newWithElement(newWithMapValue(source())); + PropertyValues values = newWithElement(newWithMapValue(source())); pv.addPropertyValues(values); sink(getMapKey(getElement(pv))); // Safe sink(getMapValue(getElement(pv))); // $hasValueFlow @@ -234,46 +240,46 @@ public class Test { // "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(PropertyValues);;MapValue of Element of Argument[0];MapValue of Element of ReturnValue;value", { MutablePropertyValues pv = new MutablePropertyValues(); - PropertyValues values = (PropertyValues) newWithElement(newWithMapValue(source())); + PropertyValues values = newWithElement(newWithMapValue(source())); PropertyValues pv2 = pv.addPropertyValues(values); sink(getMapKey(getElement(pv2))); // Safe sink(getMapValue(getElement(pv2))); // $hasValueFlow } // "org.springframework.beans;MutablePropertyValues;true;get;;;MapValue of Element of Argument[-1];ReturnValue;value", { - MutablePropertyValues pv = (MutablePropertyValues) newWithElement(newWithMapValue(source())); + MutablePropertyValues pv = newWithElement(newWithMapValue(source())); sink(pv.get("something")); // $hasValueFlow } // "org.springframework.beans;MutablePropertyValues;true;getPropertyValue;;;Element of Argument[-1];ReturnValue;value", { - MutablePropertyValues pv1 = (MutablePropertyValues) newWithElement(newWithMapKey(source())); + MutablePropertyValues pv1 = newWithElement(newWithMapKey(source())); sink(pv1.getPropertyValue("something").getName()); // $hasValueFlow sink(pv1.getPropertyValue("something").getValue()); // Safe - MutablePropertyValues pv2 = (MutablePropertyValues) newWithElement(newWithMapValue(source())); + MutablePropertyValues pv2 = newWithElement(newWithMapValue(source())); sink(pv2.getPropertyValue("something").getName()); // Safe sink(pv2.getPropertyValue("something").getValue()); // $hasValueFlow } // "org.springframework.beans;MutablePropertyValues;true;getPropertyValueList;;;Element of Argument[-1];Element of ReturnValue;value", { - MutablePropertyValues pv1 = (MutablePropertyValues) newWithElement(newWithMapKey(source())); + MutablePropertyValues pv1 = newWithElement(newWithMapKey(source())); List pvl1 = pv1.getPropertyValueList(); sink(getMapKey(getElement(pvl1))); // $hasValueFlow sink(getMapValue(getElement(pvl1))); // Safe - MutablePropertyValues pv2 = (MutablePropertyValues) newWithElement(newWithMapValue(source())); + MutablePropertyValues pv2 = newWithElement(newWithMapValue(source())); List pvl2 = pv2.getPropertyValueList(); sink(getMapKey(getElement(pvl2))); // Safe sink(getMapValue(getElement(pvl2))); // $hasValueFlow } // "org.springframework.beans;MutablePropertyValues;true;getPropertyValues;;;Element of Argument[-1];ArrayElement of ReturnValue;value", { - MutablePropertyValues pv1 = (MutablePropertyValues) newWithElement(newWithMapKey(source())); + MutablePropertyValues pv1 = newWithElement(newWithMapKey(source())); PropertyValue[] pvl1 = pv1.getPropertyValues(); sink(getMapKey(getArrayElement(pvl1))); // $hasValueFlow sink(getMapValue(getArrayElement(pvl1))); // Safe - MutablePropertyValues pv2 = (MutablePropertyValues) newWithElement(newWithMapValue(source())); + MutablePropertyValues pv2 = newWithElement(newWithMapValue(source())); PropertyValue[] pvl2 = pv2.getPropertyValues(); sink(getMapKey(getArrayElement(pvl2))); // Safe sink(getMapValue(getArrayElement(pvl2))); // $hasValueFlow @@ -281,13 +287,13 @@ public class Test { // "org.springframework.beans;MutablePropertyValues;true;setPropertyValueAt;;;Argument[0];Element of Argument[-1];value" { MutablePropertyValues pv1 = new MutablePropertyValues(); - PropertyValue v1 = (PropertyValue) newWithMapKey(source()); + PropertyValue v1 = newWithMapKey(source()); pv1.setPropertyValueAt(v1, 0); sink(getMapKey(getElement(pv1))); // $hasValueFlow sink(getMapValue(getElement(pv1))); // Safe MutablePropertyValues pv2 = new MutablePropertyValues(); - PropertyValue v2 = (PropertyValue) newWithMapValue(source()); + PropertyValue v2 = newWithMapValue(source()); pv2.setPropertyValueAt(v2, 0); sink(getMapKey(getElement(pv2))); // Safe sink(getMapValue(getElement(pv2))); // $hasValueFlow diff --git a/java/ql/test/library-tests/frameworks/spring/beans/test.ql b/java/ql/test/library-tests/frameworks/spring/beans/test.ql index 05800dbc1f2..912675470e1 100644 --- a/java/ql/test/library-tests/frameworks/spring/beans/test.ql +++ b/java/ql/test/library-tests/frameworks/spring/beans/test.ql @@ -3,22 +3,6 @@ import semmle.code.java.dataflow.ExternalFlow import semmle.code.java.dataflow.TaintTracking import TestUtilities.InlineExpectationsTest -class SummaryModelTest extends SummaryModelCsv { - override predicate row(string row) { - row = - [ - //"package;type;overrides;name;signature;ext;inputspec;outputspec;kind", - "generatedtest;Test;false;getMapKey;;;MapKey of Argument[0];ReturnValue;value", - "generatedtest;Test;false;getMapValue;;;MapValue of Argument[0];ReturnValue;value", - "generatedtest;Test;false;getElement;;;Element of Argument[0];ReturnValue;value", - "generatedtest;Test;false;getArrayElement;;;ArrayElement of Argument[0];ReturnValue;value", - "generatedtest;Test;false;newWithElement;;;Argument[0];Element of ReturnValue;value", - "generatedtest;Test;false;newWithMapKey;;;Argument[0];MapKey of ReturnValue;value", - "generatedtest;Test;false;newWithMapValue;;;Argument[0];MapValue of ReturnValue;value" - ] - } -} - class ValueFlowConf extends DataFlow::Configuration { ValueFlowConf() { this = "qltest:valueFlowConf" } From b64b8ecec2abc4219b012da13f2dfe54eda83ddf Mon Sep 17 00:00:00 2001 From: Tony Torralba Date: Wed, 30 Jun 2021 09:52:22 +0200 Subject: [PATCH 7/9] Apply suggestions from code review Co-authored-by: Anders Schack-Mulligen --- .../code/java/frameworks/spring/SpringBeans.qll | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/java/ql/src/semmle/code/java/frameworks/spring/SpringBeans.qll b/java/ql/src/semmle/code/java/frameworks/spring/SpringBeans.qll index 74c56025fa7..b6dea1ff240 100644 --- a/java/ql/src/semmle/code/java/frameworks/spring/SpringBeans.qll +++ b/java/ql/src/semmle/code/java/frameworks/spring/SpringBeans.qll @@ -4,7 +4,7 @@ */ import java -import semmle.code.java.dataflow.ExternalFlow +private import semmle.code.java.dataflow.ExternalFlow /** * Provides models for the `org.springframework.beans` package. @@ -23,21 +23,17 @@ private class FlowSummaries extends SummaryModelCsv { "org.springframework.beans;PropertyValues;true;getPropertyValue;;;MapValue of Element of Argument[-1];ReturnValue;value", "org.springframework.beans;PropertyValues;true;getPropertyValues;;;Element of Argument[-1];ArrayElement of ReturnValue;value", "org.springframework.beans;MutablePropertyValues;true;add;(String,Object);;Argument[0];MapKey of Element of Argument[-1];value", - "org.springframework.beans;MutablePropertyValues;true;add;(String,Object);;Argument[0];MapKey of Element of ReturnValue;value", + "org.springframework.beans;MutablePropertyValues;true;add;(String,Object);;Argument[-1];ReturnValue;value", "org.springframework.beans;MutablePropertyValues;true;add;(String,Object);;Argument[1];MapValue of Element of Argument[-1];value", - "org.springframework.beans;MutablePropertyValues;true;add;(String,Object);;Argument[1];MapValue of Element of ReturnValue;value", "org.springframework.beans;MutablePropertyValues;true;addPropertyValue;(PropertyValue);;Argument[0];Element of Argument[-1];value", - "org.springframework.beans;MutablePropertyValues;true;addPropertyValue;(PropertyValue);;Argument[0];Element of ReturnValue;value", + "org.springframework.beans;MutablePropertyValues;true;addPropertyValue;(PropertyValue);;Argument[-1];ReturnValue;value", "org.springframework.beans;MutablePropertyValues;true;addPropertyValue;(String,Object);;Argument[0];MapKey of Element of Argument[-1];value", "org.springframework.beans;MutablePropertyValues;true;addPropertyValue;(String,Object);;Argument[1];MapValue of Element of Argument[-1];value", "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(Map);;MapKey of Argument[0];MapKey of Element of Argument[-1];value", - "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(Map);;MapKey of Argument[0];MapKey of Element of ReturnValue;value", "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(Map);;MapValue of Argument[0];MapValue of Element of Argument[-1];value", - "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(Map);;MapValue of Argument[0];MapValue of Element of ReturnValue;value", - "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(PropertyValues);;MapKey of Element of Argument[0];MapKey of Element of Argument[-1];value", - "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(PropertyValues);;MapKey of Element of Argument[0];MapKey of Element of ReturnValue;value", - "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(PropertyValues);;MapValue of Element of Argument[0];MapValue of Element of Argument[-1];value", - "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(PropertyValues);;MapValue of Element of Argument[0];MapValue of Element of ReturnValue;value", + "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(Map);;Argument[-1];ReturnValue;value", + "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(PropertyValues);;Element of Argument[0];Element of Argument[-1];value", + "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(PropertyValues);;Argument[-1];ReturnValue;value", "org.springframework.beans;MutablePropertyValues;true;get;;;MapValue of Element of Argument[-1];ReturnValue;value", "org.springframework.beans;MutablePropertyValues;true;getPropertyValue;;;Element of Argument[-1];ReturnValue;value", "org.springframework.beans;MutablePropertyValues;true;getPropertyValueList;;;Element of Argument[-1];Element of ReturnValue;value", From 9d64cadb5067f0fff265ee9e2c6868e8505e2451 Mon Sep 17 00:00:00 2001 From: Tony Torralba Date: Wed, 30 Jun 2021 10:02:03 +0200 Subject: [PATCH 8/9] Adapt tests after applying changes from code review --- .../java/frameworks/spring/SpringBeans.qll | 2 +- .../frameworks/spring/beans/Test.java | 56 +++++++++---------- 2 files changed, 27 insertions(+), 31 deletions(-) diff --git a/java/ql/src/semmle/code/java/frameworks/spring/SpringBeans.qll b/java/ql/src/semmle/code/java/frameworks/spring/SpringBeans.qll index b6dea1ff240..7833f5668db 100644 --- a/java/ql/src/semmle/code/java/frameworks/spring/SpringBeans.qll +++ b/java/ql/src/semmle/code/java/frameworks/spring/SpringBeans.qll @@ -20,7 +20,7 @@ private class FlowSummaries extends SummaryModelCsv { "org.springframework.beans;PropertyValue;false;PropertyValue;(PropertyValue,Object);;Argument[1];MapValue of Argument[-1];value", "org.springframework.beans;PropertyValue;false;getName;;;MapKey of Argument[-1];ReturnValue;value", "org.springframework.beans;PropertyValue;false;getValue;;;MapValue of Argument[-1];ReturnValue;value", - "org.springframework.beans;PropertyValues;true;getPropertyValue;;;MapValue of Element of Argument[-1];ReturnValue;value", + "org.springframework.beans;PropertyValues;true;getPropertyValue;;;Element of Argument[-1];ReturnValue;value", "org.springframework.beans;PropertyValues;true;getPropertyValues;;;Element of Argument[-1];ArrayElement of ReturnValue;value", "org.springframework.beans;MutablePropertyValues;true;add;(String,Object);;Argument[0];MapKey of Element of Argument[-1];value", "org.springframework.beans;MutablePropertyValues;true;add;(String,Object);;Argument[-1];ReturnValue;value", diff --git a/java/ql/test/library-tests/frameworks/spring/beans/Test.java b/java/ql/test/library-tests/frameworks/spring/beans/Test.java index 7132f10850c..945dfffc144 100644 --- a/java/ql/test/library-tests/frameworks/spring/beans/Test.java +++ b/java/ql/test/library-tests/frameworks/spring/beans/Test.java @@ -97,7 +97,7 @@ public class Test { sink(v.getName()); // Safe sink(v.getValue()); // $hasValueFlow } - // "org.springframework.beans;PropertyValues;true;getPropertyValue;;;MapValue of Element of Argument[-1];ReturnValue;value", + // "org.springframework.beans;PropertyValues;true;getPropertyValue;;;Element of Argument[-1];ReturnValue;value", { PropertyValues pv = newWithElement(newWithMapValue(source())); sink(pv.getPropertyValue("safe").getValue()); // $hasValueFlow @@ -116,12 +116,17 @@ public class Test { sink(getMapKey(getElement(pv))); // $hasValueFlow sink(getMapValue(getElement(pv))); // Safe } - // "org.springframework.beans;MutablePropertyValues;true;add;(String,Object);;Argument[0];MapKey of Element of ReturnValue;value", + // "org.springframework.beans;MutablePropertyValues;true;add;(String,Object);;Argument[-1];ReturnValue;value", { MutablePropertyValues pv = new MutablePropertyValues(); sink(getMapKey(getElement(pv.add(source(), null)))); // $hasValueFlow sink(getMapValue(getElement(pv.add(source(), null)))); // Safe } + { + MutablePropertyValues pv = new MutablePropertyValues(); + sink(getMapKey(getElement(pv.add("safe", source())))); // Safe + sink(getMapValue(getElement(pv.add("safe", source())))); // $hasValueFlow + } // "org.springframework.beans;MutablePropertyValues;true;add;(String,Object);;Argument[1];MapValue of Element of Argument[-1];value", { MutablePropertyValues pv = new MutablePropertyValues(); @@ -129,12 +134,6 @@ public class Test { sink(getMapKey(getElement(pv))); // Safe sink(getMapValue(getElement(pv))); // $hasValueFlow } - // "org.springframework.beans;MutablePropertyValues;true;add;(String,Object);;Argument[1];MapValue of Element of ReturnValue;value", - { - MutablePropertyValues pv = new MutablePropertyValues(); - sink(getMapKey(getElement(pv.add("safe", source())))); // Safe - sink(getMapValue(getElement(pv.add("safe", source())))); // $hasValueFlow - } // "org.springframework.beans;MutablePropertyValues;true;addPropertyValue;(PropertyValue);;Argument[0];Element of Argument[-1];value", { MutablePropertyValues pv1 = new MutablePropertyValues(); @@ -149,7 +148,7 @@ public class Test { sink(getMapKey(getElement(pv2))); // Safe sink(getMapValue(getElement(pv2))); // $hasValueFlow } - // "org.springframework.beans;MutablePropertyValues;true;addPropertyValue;(PropertyValue);;Argument[0];Element of ReturnValue;value", + // "org.springframework.beans;MutablePropertyValues;true;addPropertyValue;(PropertyValue);;Argument[-1];ReturnValue;value", { MutablePropertyValues pv1 = new MutablePropertyValues(); PropertyValue v1 = newWithMapKey(source()); @@ -186,15 +185,6 @@ public class Test { sink(getMapKey(getElement(pv))); // $hasValueFlow sink(getMapValue(getElement(pv))); // Safe } - // "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(Map);;MapKey of Argument[0];MapKey of Element of ReturnValue;value", - { - MutablePropertyValues pv = new MutablePropertyValues(); - Map values = new HashMap(); - values.put(source(), null); - PropertyValues pv2 = pv.addPropertyValues(values); - sink(getMapKey(getElement(pv2))); // $hasValueFlow - sink(getMapValue(getElement(pv2))); // Safe - } // "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(Map);;MapValue of Argument[0];MapValue of Element of Argument[-1];value", { MutablePropertyValues pv = new MutablePropertyValues(); @@ -204,7 +194,7 @@ public class Test { sink(getMapKey(getElement(pv))); // Safe sink(getMapValue(getElement(pv))); // $hasValueFlow } - // "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(Map);;MapValue of Argument[0];MapValue of Element of ReturnValue;value", + // "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(Map);;Argument[-1];ReturnValue;value", { MutablePropertyValues pv = new MutablePropertyValues(); Map values = new HashMap(); @@ -213,7 +203,15 @@ public class Test { sink(getMapKey(getElement(pv2))); // Safe sink(getMapValue(getElement(pv2))); // $hasValueFlow } - // "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(PropertyValues);;MapKey of Element of Argument[0];MapKey of Element of Argument[-1];value", + { + MutablePropertyValues pv = new MutablePropertyValues(); + Map values = new HashMap(); + values.put(source(), null); + PropertyValues pv2 = pv.addPropertyValues(values); + sink(getMapKey(getElement(pv2))); // $hasValueFlow + sink(getMapValue(getElement(pv2))); // Safe + } + // "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(PropertyValues);;Element of Argument[0];Element of Argument[-1];value", { MutablePropertyValues pv = new MutablePropertyValues(); PropertyValues values = newWithElement(newWithMapKey(source())); @@ -221,15 +219,6 @@ public class Test { sink(getMapKey(getElement(pv))); // $hasValueFlow sink(getMapValue(getElement(pv))); // Safe } - // "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(PropertyValues);;MapKey of Element of Argument[0];MapKey of Element of ReturnValue;value", - { - MutablePropertyValues pv = new MutablePropertyValues(); - PropertyValues values = newWithElement(newWithMapKey(source())); - PropertyValues pv2 = pv.addPropertyValues(values); - sink(getMapKey(getElement(pv2))); // $hasValueFlow - sink(getMapValue(getElement(pv2))); // Safe - } - // "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(PropertyValues);;MapValue of Element of Argument[0];MapValue of Element of Argument[-1];value", { MutablePropertyValues pv = new MutablePropertyValues(); PropertyValues values = newWithElement(newWithMapValue(source())); @@ -237,7 +226,14 @@ public class Test { sink(getMapKey(getElement(pv))); // Safe sink(getMapValue(getElement(pv))); // $hasValueFlow } - // "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(PropertyValues);;MapValue of Element of Argument[0];MapValue of Element of ReturnValue;value", + // "org.springframework.beans;MutablePropertyValues;true;addPropertyValues;(PropertyValues);;Argument[-1];ReturnValue;value", + { + MutablePropertyValues pv = new MutablePropertyValues(); + PropertyValues values = newWithElement(newWithMapKey(source())); + PropertyValues pv2 = pv.addPropertyValues(values); + sink(getMapKey(getElement(pv2))); // $hasValueFlow + sink(getMapValue(getElement(pv2))); // Safe + } { MutablePropertyValues pv = new MutablePropertyValues(); PropertyValues values = newWithElement(newWithMapValue(source())); From a3e1b139c35f2db2a8d8a6d0408136eb74a41832 Mon Sep 17 00:00:00 2001 From: Tony Torralba Date: Wed, 30 Jun 2021 12:56:45 +0200 Subject: [PATCH 9/9] Fix spring stubs location --- java/ql/test/library-tests/frameworks/spring/beans/options | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/ql/test/library-tests/frameworks/spring/beans/options b/java/ql/test/library-tests/frameworks/spring/beans/options index 31b8e3f6935..0c6ef357b21 100644 --- a/java/ql/test/library-tests/frameworks/spring/beans/options +++ b/java/ql/test/library-tests/frameworks/spring/beans/options @@ -1 +1 @@ -//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/springframework-5.2.3 \ No newline at end of file +//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/springframework-5.3.8 \ No newline at end of file