-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Java: model Spring web.method.support #6595
base: main
Are you sure you want to change the base?
Changes from 5 commits
8068207
fce8f41
7b2815a
6682d93
8cf9e9a
84f24c8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
/** Provides models of taint flow in `org.springframework.web.method` */ | ||
|
||
import java | ||
private import semmle.code.java.dataflow.ExternalFlow | ||
|
||
// currently only models classes in the `support` subpackage | ||
private class FlowSummaries extends SummaryModelCsv { | ||
override predicate row(string row) { | ||
row = | ||
[ | ||
// for review: arguably this shouldn't be modeled as the implementations of resolveArgument that I've seen are effectively sanitized | ||
"org.springframework.web.method.support;HandlerMethodArgumentResolver;true;resolveArgument;;;Argument[2];ReturnValue;taint", | ||
"org.springframework.web.method.support;UriComponentsContributor;true;contributeMethodArgument;;;Argument[1];Argument[2];taint", | ||
"org.springframework.web.method.support;UriComponentsContributor;true;contributeMethodArgument;;;Argument[1];Argument[3];taint", | ||
// InvocableHandlerMethod is not modeled as it is difficult to model method-like classes with CSV | ||
// This is a very broad definition of data flow; there is a method `setRedirectModelScenario(boolean)` which is used to determine which of the `Default` and `Redirect` models are returned by `getModel`, and the methods that deal with attributes below are convenience methods for `.getMethod().*`. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. .getModel().*? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. e.g.
sauyon marked this conversation as resolved.
Show resolved
Hide resolved
|
||
"org.springframework.web.method.support;ModelAndViewContainer;false;getModel;;;SyntheticField[org.springframework.web.method.support.ModelAndViewContainer.Model] of Argument[-1];ReturnValue;value", | ||
"org.springframework.web.method.support;ModelAndViewContainer;false;getDefaultModel;;;SyntheticField[org.springframework.web.method.support.ModelAndViewContainer.Model] of Argument[-1];ReturnValue;value", | ||
"org.springframework.web.method.support;ModelAndViewContainer;false;setRedirectModel;;;Argument[0];SyntheticField[org.springframework.web.method.support.ModelAndViewContainer.Model] of Argument[-1];value", | ||
"org.springframework.web.method.support;ModelAndViewContainer;false;addAttribute;;;Argument[0];MapKey of SyntheticField[org.springframework.web.method.support.ModelAndViewContainer.Model] of Argument[-1];value", | ||
"org.springframework.web.method.support;ModelAndViewContainer;false;addAttribute;;;Argument[1];MapValue of SyntheticField[org.springframework.web.method.support.ModelAndViewContainer.Model] of Argument[-1];value", | ||
"org.springframework.web.method.support;ModelAndViewContainer;false;addAllAttributes;;;MapKey of Argument[0];MapKey of SyntheticField[org.springframework.web.method.support.ModelAndViewContainer.Model] of Argument[-1];value", | ||
"org.springframework.web.method.support;ModelAndViewContainer;false;addAllAttributes;;;MapValue of Argument[0];MapValue of SyntheticField[org.springframework.web.method.support.ModelAndViewContainer.Model] of Argument[-1];value", | ||
"org.springframework.web.method.support;ModelAndViewContainer;false;mergeAttributes;;;MapKey of Argument[0];MapKey of SyntheticField[org.springframework.web.method.support.ModelAndViewContainer.Model] of Argument[-1];value", | ||
"org.springframework.web.method.support;ModelAndViewContainer;false;mergeAttributes;;;MapValue of Argument[0];MapValue of SyntheticField[org.springframework.web.method.support.ModelAndViewContainer.Model] of Argument[-1];value", | ||
"org.springframework.web.method.support;ModelAndViewContainer;false;setView;;;Argument[0];SyntheticField[org.springframework.web.method.support.ModelAndViewContainer.View] of Argument[-1];value", | ||
"org.springframework.web.method.support;ModelAndViewContainer;false;getView;;;SyntheticField[org.springframework.web.method.support.ModelAndViewContainer.View] of Argument[-1];ReturnValue;value" | ||
] | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,136 @@ | ||
package generatedtest; | ||
|
||
import java.util.Map; | ||
import org.springframework.ui.ModelMap; | ||
import org.springframework.web.context.request.NativeWebRequest; | ||
import org.springframework.web.method.support.HandlerMethodArgumentResolver; | ||
import org.springframework.web.method.support.ModelAndViewContainer; | ||
import org.springframework.web.method.support.UriComponentsContributor; | ||
import org.springframework.web.util.UriComponentsBuilder; | ||
|
||
// Test case generated by GenerateFlowTestCase.ql | ||
public class MethodTest { | ||
|
||
Object getMapKey(Map container) { return container.keySet().iterator().next(); } | ||
Object getMapValue(Map container) { return container.get(null); } | ||
ModelMap getModelAndViewContainer_Model(ModelAndViewContainer container) { return container.getModel(); } | ||
Object getModelAndViewContainer_View(ModelAndViewContainer container) { return container.getView(); } | ||
ModelAndViewContainer newWithModelAndViewContainer_Model(Object element) { ModelAndViewContainer ret = new ModelAndViewContainer(); ret.setRedirectModel((ModelMap)element); return ret; } | ||
ModelAndViewContainer newWithModelAndViewContainer_View(Object element) { ModelAndViewContainer ret = new ModelAndViewContainer(); ret.setView(element); return ret; } | ||
Object source() { return null; } | ||
void sink(Object o) { } | ||
|
||
public void test() throws Exception { | ||
|
||
{ | ||
// "org.springframework.web.method.support;HandlerMethodArgumentResolver;true;resolveArgument;;;Argument[2];ReturnValue;taint" | ||
Object out = null; | ||
NativeWebRequest in = (NativeWebRequest)source(); | ||
HandlerMethodArgumentResolver instance = null; | ||
out = instance.resolveArgument(null, null, in, null); | ||
sink(out); // $ hasTaintFlow | ||
} | ||
{ | ||
// "org.springframework.web.method.support;ModelAndViewContainer;false;addAllAttributes;;;MapKey of Argument[0];MapKey of SyntheticField[org.springframework.web.method.support.ModelAndViewContainer.Model] of Argument[-1];value" | ||
ModelAndViewContainer out = null; | ||
Map in = Map.of(source(), null); | ||
out.addAllAttributes(in); | ||
sink(getMapKey(getModelAndViewContainer_Model(out))); // $ hasValueFlow | ||
} | ||
{ | ||
// "org.springframework.web.method.support;ModelAndViewContainer;false;addAllAttributes;;;MapValue of Argument[0];MapValue of SyntheticField[org.springframework.web.method.support.ModelAndViewContainer.Model] of Argument[-1];value" | ||
ModelAndViewContainer out = null; | ||
Map in = Map.of(null, source()); | ||
out.addAllAttributes(in); | ||
sink(getMapValue(getModelAndViewContainer_Model(out))); // $ hasValueFlow | ||
} | ||
{ | ||
// "org.springframework.web.method.support;ModelAndViewContainer;false;addAttribute;;;Argument[0];MapKey of SyntheticField[org.springframework.web.method.support.ModelAndViewContainer.Model] of Argument[-1];value" | ||
ModelAndViewContainer out = null; | ||
String in = (String)source(); | ||
out.addAttribute(in, null); | ||
sink(getMapKey(getModelAndViewContainer_Model(out))); // $ hasValueFlow | ||
} | ||
{ | ||
// "org.springframework.web.method.support;ModelAndViewContainer;false;addAttribute;;;Argument[0];MapKey of SyntheticField[org.springframework.web.method.support.ModelAndViewContainer.Model] of Argument[-1];value" | ||
ModelAndViewContainer out = null; | ||
Object in = (Object)source(); | ||
out.addAttribute(in); | ||
sink(getMapKey(getModelAndViewContainer_Model(out))); // $ hasValueFlow | ||
} | ||
{ | ||
// "org.springframework.web.method.support;ModelAndViewContainer;false;addAttribute;;;Argument[1];MapValue of SyntheticField[org.springframework.web.method.support.ModelAndViewContainer.Model] of Argument[-1];value" | ||
ModelAndViewContainer out = null; | ||
Object in = (Object)source(); | ||
out.addAttribute(null, in); | ||
sink(getMapValue(getModelAndViewContainer_Model(out))); // $ hasValueFlow | ||
} | ||
{ | ||
// "org.springframework.web.method.support;ModelAndViewContainer;false;getDefaultModel;;;SyntheticField[org.springframework.web.method.support.ModelAndViewContainer.Model] of Argument[-1];ReturnValue;value" | ||
ModelMap out = null; | ||
ModelAndViewContainer in = (ModelAndViewContainer)newWithModelAndViewContainer_Model(source()); | ||
out = in.getDefaultModel(); | ||
sink(out); // $ hasValueFlow | ||
} | ||
{ | ||
// "org.springframework.web.method.support;ModelAndViewContainer;false;getModel;;;SyntheticField[org.springframework.web.method.support.ModelAndViewContainer.Model] of Argument[-1];ReturnValue;value" | ||
ModelMap out = null; | ||
ModelAndViewContainer in = (ModelAndViewContainer)newWithModelAndViewContainer_Model(source()); | ||
out = in.getModel(); | ||
sink(out); // $ hasValueFlow | ||
} | ||
{ | ||
// "org.springframework.web.method.support;ModelAndViewContainer;false;getView;;;SyntheticField[org.springframework.web.method.support.ModelAndViewContainer.View] of Argument[-1];ReturnValue;value" | ||
Object out = null; | ||
ModelAndViewContainer in = (ModelAndViewContainer)newWithModelAndViewContainer_View(source()); | ||
out = in.getView(); | ||
sink(out); // $ hasValueFlow | ||
} | ||
{ | ||
// "org.springframework.web.method.support;ModelAndViewContainer;false;mergeAttributes;;;MapKey of Argument[0];MapKey of SyntheticField[org.springframework.web.method.support.ModelAndViewContainer.Model] of Argument[-1];value" | ||
ModelAndViewContainer out = null; | ||
Map in = Map.of(source(), null); | ||
out.mergeAttributes(in); | ||
sink(getMapKey(getModelAndViewContainer_Model(out))); // $ hasValueFlow | ||
} | ||
{ | ||
// "org.springframework.web.method.support;ModelAndViewContainer;false;mergeAttributes;;;MapValue of Argument[0];MapValue of SyntheticField[org.springframework.web.method.support.ModelAndViewContainer.Model] of Argument[-1];value" | ||
ModelAndViewContainer out = null; | ||
Map in = Map.of(null, source()); | ||
out.mergeAttributes(in); | ||
sink(getMapValue(getModelAndViewContainer_Model(out))); // $ hasValueFlow | ||
} | ||
{ | ||
// "org.springframework.web.method.support;ModelAndViewContainer;false;setRedirectModel;;;Argument[0];SyntheticField[org.springframework.web.method.support.ModelAndViewContainer.Model] of Argument[-1];value" | ||
ModelAndViewContainer out = null; | ||
ModelMap in = (ModelMap)source(); | ||
out.setRedirectModel(in); | ||
sink(getModelAndViewContainer_Model(out)); // $ hasValueFlow | ||
} | ||
{ | ||
// "org.springframework.web.method.support;ModelAndViewContainer;false;setView;;;Argument[0];SyntheticField[org.springframework.web.method.support.ModelAndViewContainer.View] of Argument[-1];value" | ||
ModelAndViewContainer out = null; | ||
Object in = (Object)source(); | ||
out.setView(in); | ||
sink(getModelAndViewContainer_View(out)); // $ hasValueFlow | ||
} | ||
{ | ||
// "org.springframework.web.method.support;UriComponentsContributor;true;contributeMethodArgument;;;Argument[1];Argument[2];taint" | ||
UriComponentsBuilder out = null; | ||
Object in = (Object)source(); | ||
UriComponentsContributor instance = null; | ||
instance.contributeMethodArgument(null, in, out, null, null); | ||
sink(out); // $ hasTaintFlow | ||
} | ||
{ | ||
// "org.springframework.web.method.support;UriComponentsContributor;true;contributeMethodArgument;;;Argument[1];Argument[3];taint" | ||
Map out = null; | ||
Object in = (Object)source(); | ||
UriComponentsContributor instance = null; | ||
instance.contributeMethodArgument(null, in, null, out, null); | ||
sink(out); // $ hasTaintFlow | ||
} | ||
|
||
} | ||
|
||
} |
This file was deleted.
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
// Generated automatically from org.springframework.beans.BeanMetadataAttribute for testing purposes | ||
|
||
package org.springframework.beans; | ||
|
||
import org.springframework.beans.BeanMetadataElement; | ||
|
||
public class BeanMetadataAttribute implements BeanMetadataElement | ||
{ | ||
protected BeanMetadataAttribute() {} | ||
public BeanMetadataAttribute(String p0, Object p1){} | ||
public Object getSource(){ return null; } | ||
public Object getValue(){ return null; } | ||
public String getName(){ return null; } | ||
public String toString(){ return null; } | ||
public boolean equals(Object p0){ return false; } | ||
public int hashCode(){ return 0; } | ||
public void setSource(Object p0){} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
// Generated automatically from org.springframework.beans.BeanMetadataAttributeAccessor for testing purposes | ||
|
||
package org.springframework.beans; | ||
|
||
import org.springframework.beans.BeanMetadataAttribute; | ||
import org.springframework.beans.BeanMetadataElement; | ||
import org.springframework.core.AttributeAccessorSupport; | ||
|
||
public class BeanMetadataAttributeAccessor extends AttributeAccessorSupport implements BeanMetadataElement | ||
{ | ||
public BeanMetadataAttribute getMetadataAttribute(String p0){ return null; } | ||
public BeanMetadataAttributeAccessor(){} | ||
public Object getAttribute(String p0){ return null; } | ||
public Object getSource(){ return null; } | ||
public Object removeAttribute(String p0){ return null; } | ||
public void addMetadataAttribute(BeanMetadataAttribute p0){} | ||
public void setAttribute(String p0, Object p1){} | ||
public void setSource(Object p0){} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
// Generated automatically from org.springframework.beans.BeanMetadataElement for testing purposes | ||
|
||
package org.springframework.beans; | ||
|
||
|
||
public interface BeanMetadataElement | ||
{ | ||
default Object getSource(){ return null; } | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
// Generated automatically from org.springframework.beans.BeansException for testing purposes | ||
|
||
package org.springframework.beans; | ||
|
||
import org.springframework.core.NestedRuntimeException; | ||
|
||
abstract public class BeansException extends NestedRuntimeException | ||
{ | ||
protected BeansException() {} | ||
public BeansException(String p0){} | ||
public BeansException(String p0, Throwable p1){} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
// Generated automatically from org.springframework.beans.ConfigurablePropertyAccessor for testing purposes | ||
|
||
package org.springframework.beans; | ||
|
||
import org.springframework.beans.PropertyAccessor; | ||
import org.springframework.beans.PropertyEditorRegistry; | ||
import org.springframework.beans.TypeConverter; | ||
import org.springframework.core.convert.ConversionService; | ||
|
||
public interface ConfigurablePropertyAccessor extends PropertyAccessor, PropertyEditorRegistry, TypeConverter | ||
{ | ||
ConversionService getConversionService(); | ||
boolean isAutoGrowNestedPaths(); | ||
boolean isExtractOldValueForEditor(); | ||
void setAutoGrowNestedPaths(boolean p0); | ||
void setConversionService(ConversionService p0); | ||
void setExtractOldValueForEditor(boolean p0); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you give an example what sanitisation you've seen? If the user provides a form value or whatever is going to be associated with the given method parameter, will it be rendered safe for pasting into HTML output? Would it still work as SQL injection for example?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the two cases I saw in Spring data, the user-input strings were parsed into a
Sort
object, and page size and page number, neither of which I think can carry meaningful taint.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah sure, but presumably this would propagate taint when the argument type isn't restrictive like that? If so I'd say we should regard it as a taint propagator in general and sanitize depending on the particular usage (e.g. integers are already blanket sanitized by the XSS query)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that was my thinking. I'll leave this in then.