diff --git a/plugin/src/main/java/com/h3xstream/findsecbugs/injection/TaintDetector.java b/plugin/src/main/java/com/h3xstream/findsecbugs/injection/TaintDetector.java index 6c07e3a4a..370bc8045 100644 --- a/plugin/src/main/java/com/h3xstream/findsecbugs/injection/TaintDetector.java +++ b/plugin/src/main/java/com/h3xstream/findsecbugs/injection/TaintDetector.java @@ -27,6 +27,7 @@ import edu.umd.cs.findbugs.Detector; import edu.umd.cs.findbugs.Priorities; import edu.umd.cs.findbugs.SourceLineAnnotation; +import edu.umd.cs.findbugs.ba.AnalysisContext; import edu.umd.cs.findbugs.ba.CFGBuilderException; import edu.umd.cs.findbugs.ba.ClassContext; import edu.umd.cs.findbugs.ba.DataflowAnalysisException; @@ -46,6 +47,8 @@ import java.util.List; import java.util.Map; import java.util.Set; +import org.apache.bcel.Repository; +import org.apache.bcel.classfile.JavaClass; import org.apache.bcel.classfile.Method; import org.apache.bcel.generic.ConstantPoolGen; import org.apache.bcel.generic.Instruction; @@ -114,7 +117,7 @@ private void analyzeMethod(ClassContext classContext, Method method, Collection< } SourceLineAnnotation sourceLine = SourceLineAnnotation .fromVisitedInstruction(classContext, method, handle); - checkTaintSink(getFullMethodName(cpg, invoke), fact, sourceLine, currentMethod); + checkTaintSink(cpg, invoke, fact, sourceLine, currentMethod); InjectionPoint injectionPoint = getInjectionPoint(invoke, cpg, handle, selectedSources); for (int offset : injectionPoint.getInjectableArguments()) { Taint parameterTaint = fact.getStackValue(offset); @@ -123,7 +126,6 @@ private void analyzeMethod(ClassContext classContext, Method method, Collection< continue; } BugInstance bugInstance = new BugInstance(this, injectionPoint.getBugType(), priority); - bugInstance.addClassAndMethod(classContext.getJavaClass(), method); bugInstance.addSourceLine(sourceLine); if (injectionPoint.getInjectableMethod() != null) { @@ -143,35 +145,86 @@ private static Iterator getLocationIterator(ClassContext classContext, } } - private void checkTaintSink(String calledMethod, TaintFrame fact, + private void checkTaintSink(ConstantPoolGen cpg, InvokeInstruction invoke, TaintFrame fact, SourceLineAnnotation sourceLine, String currentMethod) throws DataflowAnalysisException { - if (methodsWithSinks.containsKey(calledMethod)) { - Set sinks = methodsWithSinks.get(calledMethod); - for (TaintSink sink : sinks) { - Taint sinkTaint = sink.getTaint(); - Set taintParameters = sinkTaint.getParameters(); - Taint finalTaint = Taint.valueOf(sinkTaint.getNonParametricState()); - for (Integer offset : taintParameters) { - Taint parameterTaint = fact.getStackValue(offset); - finalTaint = Taint.merge(finalTaint, parameterTaint); - } - if (finalTaint == null) { - continue; - } + for (TaintSink sink : getSinks(cpg, invoke, fact)) { + Taint sinkTaint = sink.getTaint(); + Set taintParameters = sinkTaint.getParameters(); + Taint finalTaint = Taint.valueOf(sinkTaint.getNonParametricState()); + for (Integer offset : taintParameters) { + Taint parameterTaint = fact.getStackValue(offset); + finalTaint = Taint.merge(finalTaint, parameterTaint); + } + if (finalTaint == null) { + continue; + } + if (finalTaint.isTainted() || finalTaint.hasParameters()) { + BugInstance bugInstance = sink.getBugInstance(); + bugInstance.addSourceLine(sourceLine); + addSourceLines(finalTaint.getLocations(), bugInstance); if (finalTaint.isTainted()) { - BugInstance bugInstance = sink.getBugInstance(); bugInstance.setPriority(Priorities.HIGH_PRIORITY); - bugInstance.addSourceLine(sourceLine); - } else if (finalTaint.hasParameters()) { + } else { assert finalTaint.isUnknown(); - BugInstance bugInstance = sink.getBugInstance(); - bugInstance.addSourceLine(sourceLine); delayBugToReport(currentMethod, finalTaint, bugInstance); } } } } + private Set getSinks(ConstantPoolGen cpg, InvokeInstruction invoke, TaintFrame frame) { + String className = getInstanceClassName(cpg, invoke, frame); + String methodName = "." + invoke.getMethodName(cpg) + invoke.getSignature(cpg); + Set sinks = methodsWithSinks.get(className.concat(methodName)); + if (sinks != null) { + return sinks; + } + try { + JavaClass javaClass = Repository.lookupClass(className); + assert javaClass != null; + return getSuperSinks(javaClass, methodName); + } catch (ClassNotFoundException ex) { + AnalysisContext.reportMissingClass(ex); + } + return Collections.emptySet(); + } + + private Set getSuperSinks(JavaClass javaClass, String method) throws ClassNotFoundException { + for (JavaClass superClass : javaClass.getSuperClasses()) { + String fullMethodName = superClass.getClassName().replace('.', '/').concat(method); + Set sinks = methodsWithSinks.get(fullMethodName); + if (sinks != null) { + return sinks; + } + } + for (JavaClass interfaceClass : javaClass.getAllInterfaces()) { + String fullMethodName = interfaceClass.getClassName().replace('.', '/').concat(method); + Set sinks = methodsWithSinks.get(fullMethodName); + if (sinks != null) { + return sinks; + } + } + return Collections.emptySet(); + } + + private static String getInstanceClassName(ConstantPoolGen cpg, InvokeInstruction invoke, TaintFrame frame) { + try { + int instanceIndex = frame.getNumArgumentsIncludingObjectInstance(invoke, cpg) - 1; + if (instanceIndex != -1) { + assert instanceIndex < frame.getStackDepth(); + Taint instanceTaint = frame.getStackValue(instanceIndex); + String className = instanceTaint.getRealInstanceClassName(); + if (className != null) { + return className; + } + } + } catch (DataflowAnalysisException ex) { + assert false : ex.getMessage(); + } + String dottedClassName = invoke.getReferenceType(cpg).toString(); + return ClassName.toSlashedClassName(dottedClassName); + } + private static InjectionPoint getInjectionPoint(InvokeInstruction invoke, ConstantPoolGen cpg, InstructionHandle handle, Collection sources) { InjectionPoint injectionPoint = null; @@ -199,7 +252,9 @@ private void reportBug(BugInstance bugInstance, Taint taint, String currentMetho private void delayBugToReport(String method, Taint taint, BugInstance bug) { TaintSink taintSink = new TaintSink(taint, bug); Set sinkSet = methodsWithSinks.get(method); - if(sinkSet == null) sinkSet = new HashSet(); + if (sinkSet == null) { + sinkSet = new HashSet(); + } sinkSet.add(taintSink); methodsWithSinks.put(method, sinkSet); } @@ -248,13 +303,6 @@ private void logException(ClassContext classContext, Method method, Exception ex + classContext.getFullyQualifiedMethodName(method), ex); } - private static String getFullMethodName(ConstantPoolGen cpg, InvokeInstruction invoke) { - String dottedClassName = invoke.getReferenceType(cpg).toString(); - StringBuilder builder = new StringBuilder(ClassName.toSlashedClassName(dottedClassName)); - builder.append(".").append(invoke.getMethodName(cpg)).append(invoke.getSignature(cpg)); - return builder.toString(); - } - private static String getFullMethodName(MethodGen methodGen) { String methodNameWithSignature = methodGen.getName() + methodGen.getSignature(); String slashedClassName = methodGen.getClassName().replace('.', '/'); diff --git a/plugin/src/main/java/com/h3xstream/findsecbugs/taintanalysis/Taint.java b/plugin/src/main/java/com/h3xstream/findsecbugs/taintanalysis/Taint.java index 783b08c14..22671270b 100644 --- a/plugin/src/main/java/com/h3xstream/findsecbugs/taintanalysis/Taint.java +++ b/plugin/src/main/java/com/h3xstream/findsecbugs/taintanalysis/Taint.java @@ -17,25 +17,30 @@ */ package com.h3xstream.findsecbugs.taintanalysis; +import edu.umd.cs.findbugs.ba.AnalysisContext; +import edu.umd.cs.findbugs.util.ClassName; import java.util.Collections; import java.util.HashSet; +import java.util.Objects; import java.util.Set; +import org.apache.bcel.generic.ObjectType; /** - * Representation of taint dataflow facts (dataflow values) for each slot - * in {@link TaintFrame} - * + * Representation of taint dataflow facts (dataflow values) for each slot in + * {@link TaintFrame} + * * @author David Formanek (Y Soft Corporation, a.s.) */ public class Taint { public enum State { + TAINTED(false, true, false), UNKNOWN(false, false, true), SAFE(true, false, false), NULL(true, false, false), INVALID(false, false, false); - + private final boolean isSafe; private final boolean isTainted; private final boolean isUnknown; @@ -45,7 +50,7 @@ public enum State { this.isTainted = isTainted; this.isUnknown = isUnknown; } - + public static State merge(State a, State b) { if (a == null || b == null) { throw new NullPointerException( @@ -68,19 +73,18 @@ public static State merge(State a, State b) { return INVALID; } } - + private State state; private static final int INVALID_INDEX = -1; private int variableIndex; private final Set taintLocations; private final Set unknownLocations; private final Set parameters; - private State nonParametricState = State.INVALID; + private State nonParametricState; + private ObjectType realInstanceClass; public Taint(State state) { - if (state == null) { - throw new NullPointerException("state is null"); - } + Objects.requireNonNull(state, "state is null"); if (state == State.INVALID) { throw new IllegalArgumentException("state not allowed"); } @@ -89,35 +93,35 @@ public Taint(State state) { this.unknownLocations = new HashSet(); this.taintLocations = new HashSet(); this.parameters = new HashSet(); + nonParametricState = State.INVALID; + this.realInstanceClass = null; } - + public Taint(Taint taint) { - if (taint == null) { - throw new NullPointerException("taint is null"); - } + Objects.requireNonNull(taint, "taint is null"); + assert taint.state != null; this.state = taint.state; this.variableIndex = taint.variableIndex; this.taintLocations = new HashSet(taint.taintLocations); this.unknownLocations = new HashSet(taint.unknownLocations); this.parameters = new HashSet(taint.getParameters()); this.nonParametricState = taint.nonParametricState; + this.realInstanceClass = taint.realInstanceClass; } - + public State getState() { assert state != null && state != State.INVALID; return state; } - - public void setState(State state) { - if (state == null) { - throw new NullPointerException("state is null"); - } + + void setState(State state) { + Objects.requireNonNull(state, "state is null"); if (state == State.INVALID) { throw new IllegalArgumentException("state not allowed to be set"); } this.state = state; } - + public int getVariableIndex() { if (variableIndex == INVALID_INDEX) { throw new IllegalStateException("index not set or has been invalidated"); @@ -125,87 +129,107 @@ public int getVariableIndex() { assert variableIndex >= 0; return variableIndex; } - + public boolean hasValidVariableIndex() { return variableIndex != INVALID_INDEX; } - - public void setVariableIndex(int index) { + + void setVariableIndex(int index) { if (index < 0) { throw new IllegalArgumentException("negative index"); } variableIndex = index; } - - public void invalidateVariableIndex() { + + void invalidateVariableIndex() { variableIndex = INVALID_INDEX; } - + public void addLocation(TaintLocation location, boolean isKnownTaintSource) { - if (location == null) { - throw new NullPointerException("location is null"); - } + Objects.requireNonNull(location, "location is null"); if (isKnownTaintSource) { - taintLocations.add(location); + taintLocations.add(location); } else { - unknownLocations.add(location); + unknownLocations.add(location); } } - + public Set getLocations() { if (taintLocations.isEmpty()) { return Collections.unmodifiableSet(unknownLocations); } return Collections.unmodifiableSet(taintLocations); } - + public boolean isSafe() { return state.isSafe; } - + public boolean isTainted() { // in context of taint analysis, null value is safe too return state.isTainted; } - + public boolean isUnknown() { return state.isUnknown; } - - public void addParameter(int parameterIndex) { + + void addParameter(int parameterIndex) { if (parameterIndex < 0) { throw new IllegalArgumentException("index cannot be negative"); } parameters.add(parameterIndex); } - + public boolean hasParameters() { return !parameters.isEmpty(); } - + public Set getParameters() { return Collections.unmodifiableSet(parameters); } - + public State getNonParametricState() { return nonParametricState; } + void setNonParametricState(State state) { + Objects.requireNonNull(state, "state is null"); + if (state == State.INVALID) { + throw new IllegalArgumentException("state not allowed to be set"); + } + nonParametricState = state; + } + + public ObjectType getRealInstanceClass() { + return realInstanceClass; + } + + void setRealInstanceClass(ObjectType objectType) { + // can be null + realInstanceClass = objectType; + } + + public String getRealInstanceClassName() { + if (realInstanceClass == null) { + return null; + } + return ClassName.toSlashedClassName(realInstanceClass.getClassName()); + } + public static Taint valueOf(String stateName) { // exceptions thrown from Enum.valueOf return valueOf(State.valueOf(stateName)); } - + public static Taint valueOf(State state) { - if (state == null) { - throw new NullPointerException("state is null"); - } + Objects.requireNonNull(state, "state is null"); if (state == State.INVALID) { return null; } return new Taint(state); } - + public static Taint merge(Taint a, Taint b) { if (a == null) { if (b == null) { @@ -236,13 +260,28 @@ public static Taint merge(Taint a, Taint b) { } else { if (b.hasParameters()) { result.nonParametricState = State.merge(a.state, b.nonParametricState); + } } + if (a.realInstanceClass != null && b.realInstanceClass != null) { + try { + if (a.realInstanceClass.equals(b.realInstanceClass) + || b.realInstanceClass.subclassOf(a.realInstanceClass)) { + result.realInstanceClass = a.realInstanceClass; + } else if (a.realInstanceClass.subclassOf(b.realInstanceClass)) { + result.realInstanceClass = b.realInstanceClass; + } + } catch (ClassNotFoundException ex) { + AnalysisContext.reportMissingClass(ex); + } } return result; } @Override public boolean equals(Object obj) { + if (this == obj) { + return true; + } if (obj == null) { return false; } @@ -255,21 +294,16 @@ public boolean equals(Object obj) { && this.taintLocations.equals(other.taintLocations) && this.unknownLocations.equals(other.unknownLocations) && this.parameters.equals(other.parameters) - && this.nonParametricState == other.nonParametricState; + && this.nonParametricState == other.nonParametricState + && Objects.equals(this.realInstanceClass, other.realInstanceClass); } @Override public int hashCode() { - int hash = 3; - hash = 31 * hash + state.hashCode(); - hash = 31 * hash + variableIndex; - hash = 31 * hash + taintLocations.hashCode(); - hash = 31 * hash + unknownLocations.hashCode(); - hash = 31 * hash + parameters.hashCode(); - hash = 31 * hash + nonParametricState.hashCode(); - return hash; + return Objects.hash(state, variableIndex, taintLocations, unknownLocations, + parameters, nonParametricState, realInstanceClass); } - + @Override public String toString() { assert state != null; diff --git a/plugin/src/main/java/com/h3xstream/findsecbugs/taintanalysis/TaintAnalysis.java b/plugin/src/main/java/com/h3xstream/findsecbugs/taintanalysis/TaintAnalysis.java index 80ec3dc5b..fdf8608d6 100644 --- a/plugin/src/main/java/com/h3xstream/findsecbugs/taintanalysis/TaintAnalysis.java +++ b/plugin/src/main/java/com/h3xstream/findsecbugs/taintanalysis/TaintAnalysis.java @@ -39,7 +39,6 @@ public class TaintAnalysis extends FrameDataflowAnalysis { private final MethodGen methodGen; private final TaintFrameModelingVisitor visitor; - private TaintMethodSummary analyzedMethodSummary; private final int parameterStackSize; public TaintAnalysis(MethodGen methodGen, DepthFirstSearch dfs, @@ -63,7 +62,6 @@ public void transferInstruction(InstructionHandle handle, BasicBlock block, Tain throws DataflowAnalysisException { visitor.setFrameAndLocation(fact, new Location(handle, block)); visitor.analyzeInstruction(handle.getInstruction()); - analyzedMethodSummary = visitor.getAnalyzedMethodSummary(); } @Override @@ -105,8 +103,11 @@ public void meetInto(TaintFrame fact, Edge edge, TaintFrame result) mergeInto(fact, result); } - public TaintMethodSummary getAnalyzedMethodSummary() { - return analyzedMethodSummary; + /** + * This method must be called after executing the data flow + */ + public void finishAnalysis() { + visitor.finishAnalysis(); } private static int getParameterStackSize(String signature, boolean isStatic) { diff --git a/plugin/src/main/java/com/h3xstream/findsecbugs/taintanalysis/TaintDataflowEngine.java b/plugin/src/main/java/com/h3xstream/findsecbugs/taintanalysis/TaintDataflowEngine.java index 6a04d431a..0f7c29098 100644 --- a/plugin/src/main/java/com/h3xstream/findsecbugs/taintanalysis/TaintDataflowEngine.java +++ b/plugin/src/main/java/com/h3xstream/findsecbugs/taintanalysis/TaintDataflowEngine.java @@ -17,14 +17,21 @@ */ package com.h3xstream.findsecbugs.taintanalysis; +import edu.umd.cs.findbugs.SystemProperties; import edu.umd.cs.findbugs.ba.CFG; import edu.umd.cs.findbugs.ba.DepthFirstSearch; import edu.umd.cs.findbugs.classfile.CheckedAnalysisException; import edu.umd.cs.findbugs.classfile.IAnalysisCache; import edu.umd.cs.findbugs.classfile.IMethodAnalysisEngine; import edu.umd.cs.findbugs.classfile.MethodDescriptor; +import java.io.BufferedWriter; +import java.io.FileNotFoundException; +import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; +import java.io.OutputStreamWriter; +import java.io.UnsupportedEncodingException; +import java.io.Writer; import org.apache.bcel.generic.MethodGen; /** @@ -37,6 +44,22 @@ public class TaintDataflowEngine implements IMethodAnalysisEngine private static final String METHODS_SUMMARIES_PATH = "taint-config/methods-summaries.txt"; private final TaintMethodSummaryMap methodSummaries = new TaintMethodSummaryMap(); + private static final boolean DEBUG_OUTPUT_SUMMARIES = SystemProperties. + getBoolean("findsecbugs.taint.outputsummaries"); + private static Writer writer = null; + + static { + if (DEBUG_OUTPUT_SUMMARIES) { + try { + writer = new BufferedWriter(new OutputStreamWriter( + new FileOutputStream("derived-summaries.txt"), "utf-8")); + } catch (UnsupportedEncodingException ex) { + assert false : ex.getMessage(); + } catch (FileNotFoundException ex) { + System.err.println(ex.getMessage()); + } + } + } public TaintDataflowEngine() { InputStream stream = null; @@ -65,14 +88,22 @@ public TaintDataflow analyze(IAnalysisCache cache, MethodDescriptor descriptor) TaintAnalysis analysis = new TaintAnalysis(methodGen, dfs, descriptor, methodSummaries); TaintDataflow flow = new TaintDataflow(cfg, analysis); flow.execute(); - TaintMethodSummary taintMethodSummary = analysis.getAnalyzedMethodSummary(); - if (taintMethodSummary.isInformative()) { - methodSummaries.put(getSlashedMethodName(methodGen), taintMethodSummary); + analysis.finishAnalysis(); + if (DEBUG_OUTPUT_SUMMARIES && writer != null) { + TaintMethodSummary derivedSummary = methodSummaries.get(getSlashedMethodName(methodGen)); + if (derivedSummary != null) { + try { + writer.append(getSlashedMethodName(methodGen) + ":" + derivedSummary + "\n"); + writer.flush(); + } catch (IOException ex) { + System.out.println("cannot write: " + ex.getMessage()); + } + } } return flow; } - private String getSlashedMethodName(MethodGen methodGen) { + private static String getSlashedMethodName(MethodGen methodGen) { String methodNameWithSignature = methodGen.getName() + methodGen.getSignature(); String slashedClassName = methodGen.getClassName().replace('.', '/'); return slashedClassName + "." + methodNameWithSignature; diff --git a/plugin/src/main/java/com/h3xstream/findsecbugs/taintanalysis/TaintFrameModelingVisitor.java b/plugin/src/main/java/com/h3xstream/findsecbugs/taintanalysis/TaintFrameModelingVisitor.java index a7f8052e2..bd9560243 100644 --- a/plugin/src/main/java/com/h3xstream/findsecbugs/taintanalysis/TaintFrameModelingVisitor.java +++ b/plugin/src/main/java/com/h3xstream/findsecbugs/taintanalysis/TaintFrameModelingVisitor.java @@ -18,6 +18,7 @@ package com.h3xstream.findsecbugs.taintanalysis; import edu.umd.cs.findbugs.ba.AbstractFrameModelingVisitor; +import edu.umd.cs.findbugs.ba.AnalysisContext; import edu.umd.cs.findbugs.ba.DataflowAnalysisException; import edu.umd.cs.findbugs.ba.InvalidBytecodeException; import edu.umd.cs.findbugs.ba.generic.GenericSignatureParser; @@ -29,6 +30,8 @@ import java.util.Iterator; import java.util.Set; import org.apache.bcel.Constants; +import org.apache.bcel.Repository; +import org.apache.bcel.classfile.JavaClass; import org.apache.bcel.generic.AALOAD; import org.apache.bcel.generic.AASTORE; import org.apache.bcel.generic.ACONST_NULL; @@ -36,7 +39,6 @@ import org.apache.bcel.generic.ARETURN; import org.apache.bcel.generic.CHECKCAST; import org.apache.bcel.generic.ConstantPoolGen; -import org.apache.bcel.generic.FieldOrMethod; import org.apache.bcel.generic.INVOKEINTERFACE; import org.apache.bcel.generic.INVOKESPECIAL; import org.apache.bcel.generic.INVOKESTATIC; @@ -46,6 +48,7 @@ import org.apache.bcel.generic.LDC2_W; import org.apache.bcel.generic.LoadInstruction; import org.apache.bcel.generic.NEW; +import org.apache.bcel.generic.ObjectType; import org.apache.bcel.generic.StoreInstruction; /** @@ -55,8 +58,6 @@ */ public class TaintFrameModelingVisitor extends AbstractFrameModelingVisitor { - private static final String TOSTRING_METHOD = "toString()Ljava/lang/String;"; - private static final String EQUALS_METHOD = "equals(Ljava/lang/Object;)Z"; private static final Set SAFE_OBJECT_TYPES; private static final Set IMMUTABLE_OBJECT_TYPES; private final MethodDescriptor methodDescriptor; @@ -98,7 +99,7 @@ public TaintFrameModelingVisitor(ConstantPoolGen cpg, MethodDescriptor method, } this.methodDescriptor = method; this.methodSummaries = methodSummaries; - this.analyzedMethodSummary = new TaintMethodSummary(); + this.analyzedMethodSummary = new TaintMethodSummary(false); } private static Collection getMutableStackIndices(String signature) { @@ -152,7 +153,9 @@ public void visitACONST_NULL(ACONST_NULL obj) { @Override public void visitNEW(NEW obj) { - pushSafe(); + Taint taint = new Taint(Taint.State.SAFE); + taint.setRealInstanceClass(obj.getLoadClassType(cpg)); + getFrame().pushValue(taint); } @Override @@ -264,6 +267,8 @@ public void visitCHECKCAST(CHECKCAST obj) { private void visitInvoke(InvokeInstruction obj) { assert obj != null; TaintMethodSummary methodSummary = getMethodSummary(obj); + ObjectType realInstanceClass = (methodSummary == null) ? + null : methodSummary.getOutputTaint().getRealInstanceClass(); Taint taint = getMethodTaint(methodSummary); assert taint != null; if (taint.isUnknown()) { @@ -271,7 +276,10 @@ private void visitInvoke(InvokeInstruction obj) { } taintMutableArguments(methodSummary, obj); transferTaintToMutables(methodSummary, taint); // adds variable index to taint too - modelInstruction(obj, getNumWordsConsumed(obj), getNumWordsProduced(obj), new Taint(taint)); + Taint taintCopy = new Taint(taint); + // return type is not the instance type always + taintCopy.setRealInstanceClass(realInstanceClass); + modelInstruction(obj, getNumWordsConsumed(obj), getNumWordsProduced(obj), taintCopy); } private TaintMethodSummary getMethodSummary(InvokeInstruction obj) { @@ -280,19 +288,16 @@ private TaintMethodSummary getMethodSummary(InvokeInstruction obj) { if (SAFE_OBJECT_TYPES.contains(returnType)) { return TaintMethodSummary.SAFE_SUMMARY; } - String className = getSlashedClassName(obj); + String className = getInstanceClassName(obj); String methodName = obj.getMethodName(cpg); - String methodNameWithSig = methodName + signature; - String fullMethodName = className + "." + methodNameWithSig; - TaintMethodSummary methodSummary = methodSummaries.get(fullMethodName); - if (methodSummary != null) { - return methodSummary; - } - if (TOSTRING_METHOD.equals(methodNameWithSig)) { - return TaintMethodSummary.DEFAULT_TOSTRING_SUMMARY; + String methodId = "." + methodName + signature; + TaintMethodSummary summary = methodSummaries.get(className.concat(methodId)); + if (summary != null) { + return summary; } - if (EQUALS_METHOD.equals(methodNameWithSig)) { - return TaintMethodSummary.DEFAULT_EQUALS_SUMMARY; + summary = getSuperMethodSummary(className, methodId); + if (summary != null) { + return summary; } if (Constants.CONSTRUCTOR_NAME.equals(methodName) && !SAFE_OBJECT_TYPES.contains("L" + className + ";")) { @@ -303,20 +308,59 @@ private TaintMethodSummary getMethodSummary(InvokeInstruction obj) { throw new InvalidBytecodeException(ex.getMessage(), ex); } } - return methodSummary; + return null; } - + + private String getInstanceClassName(InvokeInstruction invoke) { + try { + int instanceIndex = getFrame().getNumArgumentsIncludingObjectInstance(invoke, cpg) - 1; + if (instanceIndex != -1) { + assert instanceIndex < getFrame().getStackDepth(); + Taint instanceTaint = getFrame().getStackValue(instanceIndex); + String className = instanceTaint.getRealInstanceClassName(); + if (className != null) { + return className; + } + } + } catch (DataflowAnalysisException ex) { + assert false : ex.getMessage(); + } + String dottedClassName = invoke.getReferenceType(cpg).toString(); + return ClassName.toSlashedClassName(dottedClassName); + } + + private TaintMethodSummary getSuperMethodSummary(String className, String methodId) { + try { + JavaClass javaClass = Repository.lookupClass(className); + assert javaClass != null; + TaintMethodSummary summary = getSuperMethodSummary(javaClass.getSuperClasses(), methodId); + if (summary != null) { + return summary; + } + return getSuperMethodSummary(javaClass.getAllInterfaces(), methodId); + } catch (ClassNotFoundException ex) { + AnalysisContext.reportMissingClass(ex); + return null; + } + } + + private TaintMethodSummary getSuperMethodSummary(JavaClass[] javaClasses, String method) { + assert javaClasses != null; + for (JavaClass classOrInterface : javaClasses) { + String fullMethodName = classOrInterface.getClassName().replace('.', '/').concat(method); + TaintMethodSummary summary = methodSummaries.get(fullMethodName); + if (summary != null) { + return summary; + } + } + return null; + } + private static String getReturnType(String signature) { assert signature != null && signature.contains(")"); return signature.substring(signature.indexOf(')') + 1); } - private String getSlashedClassName(FieldOrMethod obj) { - assert obj != null; - String className = obj.getReferenceType(cpg).toString(); - return ClassName.toSlashedClassName(className); - } - private Taint getMethodTaint(TaintMethodSummary methodSummary) { if (methodSummary == null) { return getDefaultValue(); @@ -336,9 +380,7 @@ private Taint getMethodTaint(TaintMethodSummary methodSummary) { } private void taintMutableArguments(TaintMethodSummary methodSummary, InvokeInstruction obj) { - if (methodSummary != null - && methodSummary != TaintMethodSummary.SAFE_SUMMARY - && !Constants.CONSTRUCTOR_NAME.equals(obj.getMethodName(cpg))) { + if (methodSummary != null && methodSummary.isConfigured()) { return; } Collection mutableStackIndices = getMutableStackIndices(obj.getSignature(cpg)); @@ -351,6 +393,7 @@ private void taintMutableArguments(TaintMethodSummary methodSummary, InvokeInstr // set back the index removed during merging taint.setVariableIndex(stackValue.getVariableIndex()); } + taint.setRealInstanceClass(stackValue.getRealInstanceClass()); taint.addLocation(getTaintLocation(), false); getFrame().setValue(getFrame().getStackLocation(index), taint); setLocalVariableTaint(taint, taint); @@ -392,7 +435,10 @@ private void transferTaintToMutables(TaintMethodSummary methodSummary, Taint tai } Taint stackValue = getFrame().getStackValue(mutableStackIndex); setLocalVariableTaint(taint, stackValue); - getFrame().setValue(getFrame().getStackLocation(mutableStackIndex), new Taint(taint)); + Taint taintCopy = new Taint(taint); + // do not set instance to return values, can be different type + taintCopy.setRealInstanceClass(stackValue.getRealInstanceClass()); + getFrame().setValue(getFrame().getStackLocation(mutableStackIndex), taintCopy); } } catch (DataflowAnalysisException ex) { assert false : ex.getMessage(); // stack depth is checked @@ -433,13 +479,36 @@ public void visitARETURN(ARETURN obj) { handleNormalInstruction(obj); } - public TaintMethodSummary getAnalyzedMethodSummary() { + /** + * This method must be called from outside at the end of the method analysis + */ + public void finishAnalysis() { assert analyzedMethodSummary != null; - if (SAFE_OBJECT_TYPES.contains(getReturnType(methodDescriptor.getSignature())) - && (analyzedMethodSummary.getOutputTaint() == null - || analyzedMethodSummary.getOutputTaint().getState() != Taint.State.NULL)) { - return TaintMethodSummary.SAFE_SUMMARY; + Taint outputTaint = analyzedMethodSummary.getOutputTaint(); + if (outputTaint == null) { + // void methods + return; + } + String returnType = getReturnType(methodDescriptor.getSignature()); + if (SAFE_OBJECT_TYPES.contains(returnType) && outputTaint.getState() != Taint.State.NULL) { + // we do not have to store summaries with safe output + return; + } + String realInstanceClassName = outputTaint.getRealInstanceClassName(); + if (returnType.equals("L" + realInstanceClassName + ";")) { + // storing it in method summary is useless + outputTaint.setRealInstanceClass(null); + analyzedMethodSummary.setOuputTaint(outputTaint); + } + String className = methodDescriptor.getSlashedClassName(); + String methodId = "." + methodDescriptor.getName() + methodDescriptor.getSignature(); + if (analyzedMethodSummary.isInformative() + || getSuperMethodSummary(className, methodId) != null) { + String fullMethodName = className.concat(methodId); + if (!methodSummaries.containsKey(fullMethodName)) { + // prefer configured summaries to derived + methodSummaries.put(fullMethodName, analyzedMethodSummary); + } } - return analyzedMethodSummary; } } diff --git a/plugin/src/main/java/com/h3xstream/findsecbugs/taintanalysis/TaintMethodSummary.java b/plugin/src/main/java/com/h3xstream/findsecbugs/taintanalysis/TaintMethodSummary.java index 75dd166d0..9ad2a2320 100644 --- a/plugin/src/main/java/com/h3xstream/findsecbugs/taintanalysis/TaintMethodSummary.java +++ b/plugin/src/main/java/com/h3xstream/findsecbugs/taintanalysis/TaintMethodSummary.java @@ -32,21 +32,16 @@ public class TaintMethodSummary { private Taint outputTaint = null; private final Set mutableStackIndices = new HashSet(); - public static final TaintMethodSummary DEFAULT_TOSTRING_SUMMARY; - public static final TaintMethodSummary DEFAULT_EQUALS_SUMMARY; + private final boolean isConfigured; public static final TaintMethodSummary SAFE_SUMMARY; static { - DEFAULT_TOSTRING_SUMMARY = new TaintMethodSummary(); - DEFAULT_TOSTRING_SUMMARY.outputTaint = new Taint(Taint.State.UNKNOWN); - DEFAULT_TOSTRING_SUMMARY.outputTaint.addParameter(0); - DEFAULT_EQUALS_SUMMARY = new TaintMethodSummary(); - DEFAULT_EQUALS_SUMMARY.outputTaint = new Taint(Taint.State.UNKNOWN); - SAFE_SUMMARY = new TaintMethodSummary(); + SAFE_SUMMARY = new TaintMethodSummary(false); SAFE_SUMMARY.outputTaint = new Taint(Taint.State.SAFE); } - public TaintMethodSummary() { + public TaintMethodSummary(boolean isConfigured) { + this.isConfigured = isConfigured; } public Collection getMutableStackIndeces() { @@ -86,10 +81,10 @@ public void setOuputTaint(Taint taint) { } public static TaintMethodSummary getDefaultConstructorSummary(int stackSize) { - if (stackSize < 0) { - throw new IllegalArgumentException("negative index"); + if (stackSize < 1) { + throw new IllegalArgumentException("stack size less than 1"); } - TaintMethodSummary summary = new TaintMethodSummary(); + TaintMethodSummary summary = new TaintMethodSummary(false); summary.outputTaint = new Taint(Taint.State.UNKNOWN); summary.mutableStackIndices.add(stackSize - 1); summary.mutableStackIndices.add(stackSize); @@ -97,13 +92,14 @@ public static TaintMethodSummary getDefaultConstructorSummary(int stackSize) { } /*public static TaintMethodSummary getUnknownMethodSummary(Collection indices) { - TaintMethodSummary summary = new TaintMethodSummary(); + TaintMethodSummary summary = new TaintMethodSummary(false); summary.outputTaint = new Taint(Taint.State.UNKNOWN); summary.mutableStackIndices.addAll(indices); return summary; }*/ + public boolean isInformative() { - if (this == DEFAULT_TOSTRING_SUMMARY || this == SAFE_SUMMARY) { + if (this == SAFE_SUMMARY) { // these are loaded automatically, do not need to store them return false; } @@ -113,9 +109,19 @@ public boolean isInformative() { if (!outputTaint.isUnknown()) { return true; } - return outputTaint.hasParameters(); + if (outputTaint.hasParameters()) { + return true; + } + if (outputTaint.getRealInstanceClass() != null) { + return true; + } + return false; } + public boolean isConfigured() { + return isConfigured; + } + @Override public String toString() { if (outputTaint == null) { @@ -136,6 +142,10 @@ public String toString() { sb.append("#"); appendJoined(sb, mutableStackIndices); } + String realInstanceClassName = outputTaint.getRealInstanceClassName(); + if (realInstanceClassName != null) { + sb.append(" (").append(realInstanceClassName).append(")"); + } return sb.toString(); } @@ -162,7 +172,7 @@ public static TaintMethodSummary load(String str) throws IOException { } str = str.trim(); String[] tuple = str.split("#"); - TaintMethodSummary summary = new TaintMethodSummary(); + TaintMethodSummary summary = new TaintMethodSummary(true); if (tuple.length == 2) { str = tuple[0]; try { @@ -185,10 +195,15 @@ public static TaintMethodSummary load(String str) throws IOException { int count = tuple.length; Taint taint = new Taint(Taint.State.UNKNOWN); for (int i = 0; i < count; i++) { - try { - taint.addParameter(Integer.parseInt(tuple[i].trim())); - } catch (NumberFormatException ex) { - throw new IOException("Cannot parse parameter offset " + i, ex); + String indexOrState = tuple[i].trim(); + if (isTaintStateValue(indexOrState)) { + taint.setNonParametricState(Taint.State.valueOf(indexOrState)); + } else { + try { + taint.addParameter(Integer.parseInt(indexOrState)); + } catch (NumberFormatException ex) { + throw new IOException("Cannot parse parameter offset " + i, ex); + } } } summary.setOuputTaint(taint); diff --git a/plugin/src/main/resources/taint-config/methods-summaries.txt b/plugin/src/main/resources/taint-config/methods-summaries.txt index 80e0feddb..dd827fcc5 100644 --- a/plugin/src/main/resources/taint-config/methods-summaries.txt +++ b/plugin/src/main/resources/taint-config/methods-summaries.txt @@ -4,12 +4,10 @@ java/lang/System.getenv(Ljava/lang/String;)Ljava/lang/String;:TAINTED java/lang/System.getProperty(Ljava/lang/String;)Ljava/lang/String;:TAINTED java/lang/System.getProperty(Ljava/lang/String;Ljava/lang/String;)Ljava/lang/String;:TAINTED -java/util/Properties.getProperty(Ljava/lang/String;)Ljava/lang/String;:TAINTED -java/util/Properties.getProperty(Ljava/lang/String;Ljava/lang/String;)Ljava/lang/String;:TAINTED - javax/servlet/http/ServletRequest.getParameter(Ljava/lang/String;)Ljava/lang/String;:TAINTED javax/servlet/http/ServletRequest.getRemoteHost()Ljava/lang/String;:TAINTED +- better keep those, J2EE interfaces can be unavailable to analysis javax/servlet/http/HttpServletRequest.getParameter(Ljava/lang/String;)Ljava/lang/String;:TAINTED javax/servlet/http/HttpServletRequest.getRemoteHost()Ljava/lang/String;:TAINTED javax/servlet/http/HttpServletRequest.getHeader(Ljava/lang/String;)Ljava/lang/String;:TAINTED @@ -45,6 +43,17 @@ java/sql/ResultSet.getString(Ljava/lang/String;)Ljava/lang/String;:TAINTED +java/lang/Object.clone()Ljava/lang/Object;:0 +java/lang/Object.equals(Ljava/lang/Object;)Z:UNKNOWN + +- Not true, but allows analysis of methods calling toString on Object instances +java/lang/Object.toString()Ljava/lang/String;:0 + + +java/lang/Appendable.append(Ljava/lang/CharSequence;)Ljava/lang/Appendable;:0,1#1 +java/lang/Appendable.append(Ljava/lang/CharSequence;II)Ljava/lang/Appendable;:2,3#3 + + java/lang/StringBuilder.()V:SAFE java/lang/StringBuilder.(Ljava/lang/CharSequence;)V:0#1,2 java/lang/StringBuilder.(I)V:SAFE @@ -157,6 +166,8 @@ java/lang/String.valueOf(Ljava/lang/Object)Ljava/lang/String;:0,1 java/lang/CharSequence.subSequence(II)Ljava/lang/CharSequence;:2 +java/util/Enumeration.nextElement()Ljava/lang/Object;:0 + java/util/StringTokenizer.(Ljava/lang/String;)V:0#1,2 java/util/StringTokenizer.(Ljava/lang/String;Ljava/lang/String;)V:1#2,3 java/util/StringTokenizer.(Ljava/lang/String;Ljava/lang/String;B)V:2#3,4 @@ -195,9 +206,17 @@ java/lang/BigDecimal.toPlainString()Ljava/lang/String;:SAFE java/lang/BigDecimal.toString()Ljava/lang/String;:SAFE -java/util/Collection.add(Ljava/lang/Object;)Z:0,1#1 +- Objects from Java 8 function package are UNKNOWN by default, some could be made SAFE or TAINTED in the future +java/langIterable.forEach(Ljava/util/function/Consumer;)V:0,1#1 +java/lang/Iterable.iterator()Ljava/util/Iterator;:0 + +java/util/Iterator.next()Ljava/lang/Object;:0 + + java/util/Collection.add(Ljava/lang/Object;)Z:0,1#1 java/util/Collection.addAll(Ljava/util/Collection;)Z:0,1#1 +java/util/Collection.contains(Ljava/util/Object;)Z:UNKNOWN +java/util/Collection.containsAll(Ljava/util/Collection;)Z:UNKNOWN java/util/Collection.clear()V:SAFE#0 java/util/Collection.remove(Ljava/lang/Object;)Z:1 java/util/Collection.removeAll(Ljava/util/Collection;)Z:1 @@ -207,16 +226,15 @@ java/util/Collection.toArray()[Ljava/lang/Object;:0 java/util/Collection.toArray([Ljava/lang/Object;)[Ljava/lang/Object;:1 -java/util/List.add(Ljava/lang/Object;)Z:0,1#1 java/util/List.add(ILjava/lang/Object;)V:0,2#2 -java/util/List.addAll(Ljava/util/Collection;)Z:0,1#1 java/util/List.addAll(ILjava/util/Collection;)Z:0,2#2 -java/util/List.clear()V:SAFE#0 java/util/List.get(I)Ljava/lang/Object;:1 +java/util/List.indexOf(Ljava/lang/Object;)I:UNKNOWN +java/util/List.lastIndexOf(Ljava/lang/Object;)I:UNKNOWN +java/util/List.listIterator()Ljava/util/ListIterator;:0,UNKNOWN#0 +java/util/List.listIterator(I)Ljava/util/ListIterator;:1,UNKNOWN#1 java/util/List.remove()Ljava/lang/Object;:0 java/util/List.remove(I)Ljava/lang/Object;:1 -java/util/List.remove(Ljava/lang/Object;)Z:1 -java/util/List.removeAll(Ljava/util/Collection;)Z:1 java/util/List.replaceAll(Ljava/util/function/UnaryOperator;)V:1#1 java/util/List.retainAll(Ljava/util/Collection;)Z:1 java/util/List.set(ILjava/lang/Object;)Ljava/lang/Object;:0,2#2 @@ -224,81 +242,286 @@ java/util/List.sort(Ljava/util/Comparator;)V:1 java/util/List.subList(II)Ljava/util/List;:2#2 java/util/List.toArray()[Ljava/lang/Object;:0 java/util/List.toArray([Ljava/lang/Object;)[Ljava/lang/Object;:1 -java/util/List.removeIf(Ljava/util/function/Predicate;)Z:1#1 -java/util/List.forEach(Ljava/util/function/Consumer;)V:1 + + +- ListIterator is never SAFE, because it can modify the list and the taint is not propagated to it +java/util/ListIterator.add(Ljava/lang/Object;)V:0,1#1 +java/util/ListIterator.previous()Ljava/lang/Object;:0 +java/util/ListIterator.set(Ljava/lang/Object;)V:0,1#1 java/util/ArrayList.()V:SAFE java/util/ArrayList.(I)V:SAFE java/util/ArrayList.(Ljava/util/Collection;)V:0#1,2 -java/util/ArrayList.add(Ljava/lang/Object;)Z:0,1#1 -java/util/ArrayList.add(ILjava/lang/Object;)V:0,2#2 -java/util/ArrayList.addAll(Ljava/util/Collection;)Z:0,1#1 -java/util/ArrayList.addAll(ILjava/util/Collection;)Z:0,2#2 -java/util/ArrayList.clear()V:SAFE#0 -java/util/ArrayList.ensureCapacity(I)V:1 -java/util/ArrayList.forEach(Ljava/util/function/Consumer;)V:1 -java/util/ArrayList.get(I)Ljava/lang/Object;:1 -java/util/ArrayList.remove()Ljava/lang/Object;:0 -java/util/ArrayList.remove(I)Ljava/lang/Object;:1 -java/util/ArrayList.remove(Ljava/lang/Object;)Z:1 -java/util/ArrayList.removeAll(Ljava/util/Collection;)Z:1 -java/util/ArrayList.removeIf(Ljava/util/function/Predicate;)Z:1#1 -java/util/ArrayList.replaceAll(Ljava/util/function/UnaryOperator;)V:1#1 -java/util/ArrayList.retainAll(Ljava/util/Collection;)Z:1 -java/util/ArrayList.set(ILjava/lang/Object;)Ljava/lang/Object;:0,2#2 -java/util/ArrayList.sort(Ljava/util/Comparator;)V:1 -java/util/ArrayList.subList(II)Ljava/util/List;:2#2 -java/util/ArrayList.toArray()[Ljava/lang/Object;:0 -java/util/ArrayList.toArray([Ljava/lang/Object;)[Ljava/lang/Object;:1 -java/util/ArrayList.trimToSize()V:0 + +java/util/concurrent/CopyOnWriteArrayList.()V:SAFE +java/util/concurrent/CopyOnWriteArrayList.(Ljava/util/Collection;)V:0#1,2 +java/util/concurrent/CopyOnWriteArrayList.([Ljava/lang/Object;)V:0#1,2 + + +java/util/Queue.element()Ljava/lang/Object;:0 +java/util/Queue.offer(Ljava/lang/Object;)Z:0,1#1 +java/util/Queue.peek()Ljava/lang/Object;:0 +java/util/Queue.poll()Ljava/lang/Object;:0 +java/util/Queue.remove()Ljava/lang/Object;:0 + + +java/util/Deque.addFirst(Ljava/lang/Object;)V:0,1#1 +java/util/Deque.addLast(Ljava/lang/Object;)V:0,1#1 +java/util/Deque.descendingIterator()Ljava/util/Iterator;:0 +java/util/Deque.getFirst()Ljava/lang/Object;:0 +java/util/Deque.getLast()Ljava/lang/Object;:0 +java/util/Deque.offerFirst(Ljava/lang/Object;)Z:0,1#1 +java/util/Deque.offerLast(Ljava/lang/Object;)Z:0,1#1 +java/util/Deque.peekFirst()Ljava/lang/Object;:0 +java/util/Deque.peekLast()Ljava/lang/Object;:0 +java/util/Deque.pollFirst()Ljava/lang/Object;:0 +java/util/Deque.pollLast()Ljava/lang/Object;:0 +java/util/Deque.pop()Ljava/lang/Object;:0 +java/util/Deque.push(Ljava/lang/Object;)V:0,1#1 +java/util/Deque.removeFirst()Ljava/lang/Object;:0 +java/util/Deque.removeFirstOccurrence(Ljava/lang/Object;)Z:1 +java/util/Deque.removeLast()Ljava/lang/Object;:0 +java/util/Deque.removeLastOccurrence(Ljava/lang/Object;)Z:1 + + +java/util/ArrayDeque.()V:SAFE +java/util/ArrayDeque.(Ljava/util/Collection;)V:0#1,2 +java/util/ArrayDeque.(I)V:SAFE + +java/util/concurrent/ConcurrentLinkedDeque.()V:SAFE +java/util/concurrent/ConcurrentLinkedDeque.(Ljava/util/Collection;)V:0#1,2 java/util/LinkedList.()V:SAFE java/util/LinkedList.(Ljava/util/Collection;)V:0#1,2 -java/util/LinkedList.add(Ljava/lang/Object;)Z:0,1#1 -java/util/LinkedList.add(ILjava/lang/Object;)V:0,2#2 -java/util/LinkedList.addAll(Ljava/util/Collection;)Z:0,1#1 -java/util/LinkedList.addAll(ILjava/util/Collection;)Z:0,2#2 -java/util/LinkedList.addFirst(Ljava/lang/Object;)V:0,1#1 -java/util/LinkedList.addLast(Ljava/lang/Object;)V:0,1#1 -java/util/LinkedList.clear()V:SAFE#0 -java/util/LinkedList.element()Ljava/lang/Object;:0 java/util/LinkedList.get(I)Ljava/lang/Object;:1 -java/util/LinkedList.getFirst()Ljava/lang/Object;:0 -java/util/LinkedList.getLast()Ljava/lang/Object;:0 -java/util/LinkedList.offer(Ljava/lang/Object;)Z:0,1#1 -java/util/LinkedList.offerFirst(Ljava/lang/Object;)Z:0,1#1 -java/util/LinkedList.offerLast(Ljava/lang/Object;)Z:0,1#1 -java/util/LinkedList.peek()Ljava/lang/Object;:0 -java/util/LinkedList.peekFirst()Ljava/lang/Object;:0 -java/util/LinkedList.peekLast()Ljava/lang/Object;:0 -java/util/LinkedList.poll()Ljava/lang/Object;:0 -java/util/LinkedList.pollFirst()Ljava/lang/Object;:0 -java/util/LinkedList.pollLast()Ljava/lang/Object;:0 -java/util/LinkedList.pop()Ljava/lang/Object;:0 -java/util/LinkedList.push(Ljava/lang/Object;)V:0,1#1 -java/util/LinkedList.remove()Ljava/lang/Object;:0 -java/util/LinkedList.remove(I)Ljava/lang/Object;:1 -java/util/LinkedList.remove(Ljava/lang/Object;)Z:1 -java/util/LinkedList.removeFirstOccurrence(Ljava/lang/Object;)Z:1 -java/util/LinkedList.removeFirst()Ljava/lang/Object;:0 -java/util/LinkedList.removeLastOccurrence(Ljava/lang/Object;)Z:1 -java/util/LinkedList.removeLast()Ljava/lang/Object;:0 java/util/LinkedList.set(ILjava/lang/Object;)Ljava/lang/Object;:0,2#2 -java/util/LinkedList.toArray()[Ljava/lang/Object;:0 -java/util/LinkedList.toArray([Ljava/lang/Object;)[Ljava/lang/Object;:1 -java/util/LinkedList.subList(II)Ljava/util/List;:2#2 -java/util/LinkedList.removeAll(Ljava/util/Collection;)Z:1 -java/util/LinkedList.retainAll(Ljava/util/Collection;)Z:1 -java/util/LinkedList.replaceAll(Ljava/util/function/UnaryOperator;)V:1#1 -java/util/LinkedList.removeIf(Ljava/util/function/Predicate;)Z:1#1 + + +java/util/Vector.()V:SAFE +java/util/Vector.(Ljava/util/Collection;)V:0#1,2 +java/util/Vector.(I)V:SAFE +java/util/Vector.(II)V:SAFE +java/util/Vector.add(ILjava/lang/Object;)V:0,2#2 +java/util/Vector.addAll(ILjava/util/Collection;)Z:0,2#2 +java/util/Vector.addElement(Ljava/lang/Object;)V:0,1#1 +java/util/Vector.copyInto([Ljava/lang/Object;)V:1#0 +java/util/Vector.elementAt(I)Ljava/lang/Object;:1 +java/util/Vector.elements()Ljava/util/Enumeration;:0 +java/util/Vector.firstElement()Ljava/lang/Object;:0 +java/util/Vector.insertElementAt(Ljava/lang/Object;I)V:1,2#2 +java/util/Vector.setElementAt(Ljava/lang/Object;I)V:1,2#2 +java/util/Vector.sort(Ljava/util/Comparator;)V:UNKNOWN + + +java/util/Stack.()V:SAFE +java/util/Stack.peek()Ljava/lang/Object;:0 +java/util/Stack.pop()Ljava/lang/Object;:0 +java/util/Stack.push(Ljava/lang/Object;)Ljava/lang/Object;:0,1#1 +java/util/Stack.search(Ljava/lang/Object;)I:UNKNOWN + + +java/util/Arrays.asList([Ljava/lang/Object;)Ljava/util/List;:0 + + +- Set interface adds no new methods + +java/util/HashSet.()V:SAFE +java/util/HashSet.(Ljava/util/Collection;)V:0#1,2 +java/util/HashSet.(I)V:SAFE +java/util/HashSet.(IF)V:SAFE + +java/util/LinkedHashSet.()V:SAFE +java/util/LinkedHashSet.(Ljava/util/Collection;)V:0#1,2 +java/util/LinkedHashSet.(I)V:SAFE +java/util/LinkedHashSet.(IF)V:SAFE + + +java/util/SortedSet.first()Ljava/lang/Object;:0 +java/util/SortedSet.headSet(Ljava/lang/Object;)Ljava/util/SortedSet;:0,1 +java/util/SortedSet.last()Ljava/lang/Object;:0 +java/util/SortedSet.tailSet(Ljava/lang/Object;)Ljava/util/SortedSet;:0,1 +java/util/SortedSet.subSet(Ljava/lang/Object;Ljava/lang/Object;)Ljava/util/SortedSet;:0,1,2 + + +java/util/NavigableSet.ceiling(Ljava/lang/Object;)Ljava/lang/Object;:0,1 +java/util/NavigableSet.descendingIterator()Ljava/util/Iterator;:0 +java/util/NavigableSet.descendingSet()Ljava/util/NavigableSet;:0 +java/util/NavigableSet.floor(Ljava/lang/Object;)Ljava/lang/Object;:0,1 +java/util/NavigableSet.headSet(Ljava/lang/Object;Z)Ljava/util/NavigableSet;:1,2 +java/util/NavigableSet.higher(Ljava/lang/Object;)Ljava/lang/Object;:0,1 +java/util/NavigableSet.lower(Ljava/lang/Object;)Ljava/lang/Object;:0,1 +java/util/NavigableSet.pollFirst()Ljava/lang/Object;:0 +java/util/NavigableSet.pollLast()Ljava/lang/Object;:0 +java/util/NavigableSet.subSet(Ljava/lang/Object;ZLjava/lang/Object;Z)Ljava/util/NavigableSet;:1,3,4 +java/util/NavigableSet.tailSet(Ljava/lang/Object;Z)Ljava/util/NavigableSet;:1,2 + + +java/util/TreeSet.()V:SAFE +java/util/TreeSet.(Ljava/util/Collection;)V:0#1,2 +java/util/TreeSet.(Ljava/util/Comparator;)V:SAFE +java/util/TreeSet.(Ljava/util/SortedSet;)V:0#1,2 + + +java/util/concurrent/ConcurrentSkipListSet.()V:SAFE +java/util/concurrent/ConcurrentSkipListSet.(Ljava/util/Collection;)V:0#1,2 +java/util/concurrent/ConcurrentSkipListSet.(Ljava/util/Comparator;)V:SAFE +java/util/concurrent/ConcurrentSkipListSet.(Ljava/util/SortedSet;)V:0#1,2 + +java/util/concurrent/CopyOnWriteArraySet.()V:SAFE +java/util/concurrent/CopyOnWriteArraySet.(Ljava/util/Collection;)V:0#1,2 + + +java/util/Map.clear()V:SAFE#0 +java/util/Map.compute(Ljava/lang/Object;Ljava/util/function/BiFunction;)Ljava/lang/Object;:0,1,2#2 +java/util/Map.computeIfAbsent(Ljava/lang/Object;Ljava/util/function/BiFunction;)Ljava/lang/Object;:1,2,UNKNOWN#2 +java/util/Map.computeIfPresent(Ljava/lang/Object;Ljava/util/function/BiFunction;)Ljava/lang/Object;:1,2,UNKNOWN#2 +java/util/Map.containsKey(Ljava/lang/Object;)Z:UNKNOWN +java/util/Map.containsValue(Ljava/lang/Object;)Z:UNKNOWN +java/util/Map.entrySet()Ljava/util/Set;:0 +java/util/Map.forEach(Ljava/util/function/BiConsumer;)V:0,1#1 +java/util/Map.get(Ljava/lang/Object;)Ljava/lang/Object;:1 +java/util/Map.getOrDefault(Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;:0,2 +java/util/Map.keySet()Ljava/util/Set;:0 +java/util/Map.merge(Ljava/lang/Object;Ljava/lang/Object;Ljava/util/function/BiFunction;)Ljava/lang/Object;:0,1,2,3#3 +java/util/Map.put(Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;:0,1,2#2 +java/util/Map.putAll(Ljava/util/Map;)V:0,1#1 +java/util/Map.putIfAbsent(Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;:0,1,2#2 +java/util/Map.remove(Ljava/lang/Object;)Ljava/lang/Object;:1 +java/util/Map.remove(Ljava/lang/Object;Ljava/lang/Object;)Z:UNKNOWN +java/util/Map.replace(Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;:0,1,2#2 +java/util/Map.replace(Ljava/lang/Object;Ljava/lang/Object;Ljava/lang/Object;)Z:0,2,3#3 +java/util/Map.replaceAll(Ljava/util/function/BiFunction;)V:0,1#1 +java/util/Map.values()Ljava/util/Collection;:0 + +java/util/Map$Entry.getKey()Ljava/lang/Object;:0 +java/util/Map$Entry.getValue()Ljava/lang/Object;:0 +java/util/Map$Entry.setValue(Ljava/lang/Object;)Ljava/lang/Object;:0,1#1 + + +java/util/HashMap.()V:SAFE +java/util/HashMap.(I)V:SAFE +java/util/HashMap.(IF)V:SAFE +java/util/HashMap.(Ljava/util/Map;)V:0#1,2 + +java/util/LinkedHashMap.()V:SAFE +java/util/LinkedHashMap.(I)V:SAFE +java/util/LinkedHashMap.(IF)V:SAFE +java/util/LinkedHashMap.(IFZ)V:SAFE +java/util/LinkedHashMap.(Ljava/util/Map;)V:0#1,2 + +java/util/WeakHashMap.()V:SAFE +java/util/WeakHashMap.(I)V:SAFE +java/util/WeakHashMap.(IF)V:SAFE +java/util/WeakHashMap.(Ljava/util/Map;)V:0#1,2 + +java/util/IdentityHashMap.()V:SAFE +java/util/IdentityHashMap.(I)V:SAFE +java/util/IdentityHashMap.(Ljava/util/Map;)V:0#1,2 + +- NavigableMap and SortedMap not supported yet + +java/util/TreeMap.()V:SAFE +java/util/TreeMap.(Ljava/util/Comparator;)V:SAFE +java/util/TreeMap.(Ljava/util/Map;)V:SAFE +java/util/TreeMap.(Ljava/util/SortedMap;)V:SAFE + + +java/util/Dictionary.elements()Ljava/util/Enumeration;:0 +java/util/Dictionary.get(Ljava/lang/Object;)Ljava/lang/Object;:1 +java/util/Dictionary.keys()Ljava/util/Enumeration;:0 +java/util/Dictionary.put(Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;:0,1,2#2 +java/util/Dictionary.remove(Ljava/lang/Object;)Ljava/lang/Object;:1 + +java/util/Hashtable.()V:SAFE +java/util/Hashtable.(I)V:SAFE +java/util/Hashtable.(IF)V:SAFE +java/util/Hashtable.(Ljava/util/Map;)V:0#1,2 + + +java/util/Properties.()V:SAFE +java/util/Properties.(Ljava/util/Properties;)V:0#1,2 +java/util/Properties.getProperty(Ljava/lang/String;)Ljava/lang/String;:1 +java/util/Properties.getProperty(Ljava/lang/String;Ljava/lang/String;)Ljava/lang/String;:0,2 +java/util/Properties.load(Ljava/io/InputStream;)V:TAINTED#1 +java/util/Properties.load(Ljava/io/Reader;)V:TAINTED#1 +java/util/Properties.loadFromXML(Ljava/io/InputStream;)V:TAINTED#1 +java/util/Properties.propertyNames()Ljava/util/Enumeration;:0 +java/util/Properties.setProperty(Ljava/lang/String;Ljava/lang/String;)Ljava/lang/Object;:0,1,2#2 +java/util/Properties.stringPropertyNames()Ljava/util/Set;:0 + + +java/util/Collections.addAll(Ljava/util/Collection;[Ljava/lang/Object;)Z:0,1#1 +java/util/Collections.asLifoQueue(Ljava/util/Deque;)Ljava/util/Queue;:0 +java/util/Collections.binarySearch(Ljava/util/List;Ljava/lang/Object;)I:UNKNOWN +java/util/Collections.binarySearch(Ljava/util/List;Ljava/lang/Object;Ljava/util/Comparator;)I:UNKNOWN +java/util/Collections.checkedCollection(Ljava/util/Collection;Ljava/lang/Class;)Ljava/util/Collection;:1 +java/util/Collections.checkedList(Ljava/util/List;Ljava/lang/Class;)Ljava/util/List;:1 +java/util/Collections.checkedMap(Ljava/util/Map;Ljava/lang/Class;Ljava/lang/Class;)Ljava/util/Map;:2 +java/util/Collections.checkedNavigableMap(Ljava/util/NavigableMap;Ljava/lang/Class;Ljava/lang/Class;)Ljava/util/NavigableMap;:2 +java/util/Collections.checkedNavigableSet(Ljava/util/NavigableSet;Ljava/lang/Class;)Ljava/util/NavigableSet;:1 +java/util/Collections.checkedQueue(Ljava/util/Queue;Ljava/lang/Class;)Ljava/util/Queue;:1 +java/util/Collections.checkedSet(Ljava/util/Set;Ljava/lang/Class;)Ljava/util/Set;:1 +java/util/Collections.checkedSortedMap(Ljava/util/SortedMap;Ljava/lang/Class;Ljava/lang/Class;)Ljava/util/SortedMap;:2 +java/util/Collections.checkedSortedSet(Ljava/util/SortedSet;Ljava/lang/Class;)Ljava/util/SortedSet;:1 +java/util/Collections.copy(Ljava/util/List;Ljava/util/List;)V:0,1#1 +java/util/Collections.disjoint(Ljava/util/Collection;Ljava/util/Collection;)Z:UNKNOWN +java/util/Collections.emptyEnumeration()Ljava/util/Enumeration;:SAFE +java/util/Collections.emptyIterator()Ljava/util/Iterator;:SAFE +java/util/Collections.emptyListIterator()Ljava/util/ListIterator;:SAFE +java/util/Collections.emptyMap()Ljava/util/Map;:SAFE +java/util/Collections.emptyNavigableMap()Ljava/util/NavigableMap;:SAFE +java/util/Collections.emptyNavigableSet()Ljava/util/NavigableSet;:SAFE +java/util/Collections.emptySet()Ljava/util/Set;:SAFE +java/util/Collections.emptySortedMap()Ljava/util/SortedMap;:SAFE +java/util/Collections.emptySortedSet()Ljava/util/SortedSet;:SAFE +java/util/Collections.enumeration(Ljava/util/Collection;)Ljava/util/Enumeration;:0 +java/util/Collections.fill(Ljava/util/List;Ljava/lang/Object;)V:0#1 +java/util/Collections.frequency(Ljava/util/Collection;Ljava/lang/Object;)I:UNKNOWN +java/util/Collections.indexOfSubList(Ljava/util/List;Ljava/util/List;)I:UNKNOWN +java/util/Collections.lastIndexOfSubList(Ljava/util/List;Ljava/util/List;)I:UNKNOWN +java/util/Collections.list(Ljava/util/Enumeration;)Ljava/util/ArrayList;:0 +java/util/Collections.max(Ljava/util/Collection;)Ljava/lang/Object;:0 +java/util/Collections.max(Ljava/util/Collection;Ljava/util/Comparator;)Ljava/lang/Object;:1 +java/util/Collections.min(Ljava/util/Collection;)Ljava/lang/Object;:0 +java/util/Collections.min(Ljava/util/Collection;Ljava/util/Comparator;)Ljava/lang/Object;:1 +java/util/Collections.nCopies(ILjava/lang/Object;)Ljava/util/List;:0 +java/util/Collections.newSetFromMap(Ljava/util/Map;)Ljava/util/Set;:0 +java/util/Collections.replaceAll(Ljava/util/List;Ljava/lang/Object;Ljava/lang/Object;)Z:0,2#2 +java/util/Collections.reverse(Ljava/util/List;)V:UNKNOWN +java/util/Collections.rotate(Ljava/util/List;I)V:UNKNOWN +java/util/Collections.shuffle(Ljava/util/List;)V:UNKNOWN +java/util/Collections.shuffle(Ljava/util/List;Ljava/util/Random;)V:UNKNOWN +java/util/Collections.singleton(Ljava/lang/Object;)Ljava/util/Set;:0 +java/util/Collections.singletonList(Ljava/lang/Object;)Ljava/util/List;:0 +java/util/Collections.singletonMap(Ljava/lang/Object;Ljava/lang/Object;)Ljava/util/Map;:0,1 +java/util/Collections.sort(Ljava/util/List;)V:UNKNOWN +java/util/Collections.sort(Ljava/util/List;Ljava/util/Comparator;)V:UNKNOWN +java/util/Collections.swap(Ljava/util/List;II)V:UNKNOWN +java/util/Collections.synchronizedCollection(Ljava/util/Collection;)Ljava/util/Collection;:0 +java/util/Collections.synchronizedList(Ljava/util/List;)Ljava/util/List;:0 +java/util/Collections.synchronizedMap(Ljava/util/Map;)Ljava/util/Map;:0 +java/util/Collections.synchronizedNavigableMap(Ljava/util/NavigableMap;)Ljava/util/NavigableMap;:0 +java/util/Collections.synchronizedNavigableSet(Ljava/util/NavigableSet;)Ljava/util/NavigableSet;:0 +java/util/Collections.synchronizedSet(Ljava/util/Set;)Ljava/util/Set;:0 +java/util/Collections.synchronizedSortedMap(Ljava/util/SortedMap;)Ljava/util/SortedMap;:0 +java/util/Collections.synchronizedSortedSet(Ljava/util/SortedSet;)Ljava/util/SortedSet;:0 +java/util/Collections.unmodifiableCollection(Ljava/util/Collection;)Ljava/util/Collection;:0 +java/util/Collections.unmodifiableList(Ljava/util/List;)Ljava/util/List;:0 +java/util/Collections.unmodifiableMap(Ljava/util/Map;)Ljava/util/Map;:0 +java/util/Collections.unmodifiableNavigableMap(Ljava/util/NavigableMap;)Ljava/util/NavigableMap;:0 +java/util/Collections.unmodifiableNavigableSet(Ljava/util/NavigableSet;)Ljava/util/NavigableSet;:0 +java/util/Collections.unmodifiableSet(Ljava/util/Set;)Ljava/util/Set;:0 +java/util/Collections.unmodifiableSortedMap(Ljava/util/SortedMap;)Ljava/util/SortedMap;:0 +java/util/Collections.unmodifiableSortedSet (Ljava/util/SortedSet;)Ljava/util/SortedSet;:0 + + - The following methods receive arguments but will not alter the objects received. (It avoids tainting the parameters that are not immutable.) javax/jdo/PersistenceManager.newQuery(Ljava/lang/Object;)Ljavax/jdo/Query;:UNKNOWN javax/jdo/PersistenceManager.newQuery(Ljava/lang/String;Ljava/lang/Object;)Ljavax/jdo/Query;:UNKNOWN -java/util/Arrays.asList([Ljava/lang/Object;)Ljava/util/List;:0 java/util/logging/Logger.entering(Ljava/lang/String;Ljava/lang/String;Ljava/lang/Object;)V:UNKNOWN java/util/logging/Logger.entering(Ljava/lang/String;Ljava/lang/String;[Ljava/lang/Object;)V:UNKNOWN diff --git a/plugin/src/test/java/com/h3xstream/findsecbugs/injection/command/CommandInjectionDetectorTest.java b/plugin/src/test/java/com/h3xstream/findsecbugs/injection/command/CommandInjectionDetectorTest.java index 4848371c0..897972c9d 100644 --- a/plugin/src/test/java/com/h3xstream/findsecbugs/injection/command/CommandInjectionDetectorTest.java +++ b/plugin/src/test/java/com/h3xstream/findsecbugs/injection/command/CommandInjectionDetectorTest.java @@ -33,15 +33,20 @@ public void detectCommandInjection() throws Exception { //Locate test code String[] files = { getClassFilePath("testcode/command/CommandInjection"), - getClassFilePath("testcode/command/MoreMethods") + getClassFilePath("testcode/command/MoreMethods"), + getClassFilePath("testcode/command/SubClass") }; //Run the analysis EasyBugReporter reporter = spy(new EasyBugReporter()); analyze(files, reporter); - List linesMedium = Arrays.asList(20, 21, 22, 23, 24, 25, 29, 32, 44, 130, 135, 141, 154); - List linesHigh = Arrays.asList(73, 77, 89, 101, 111, 116, 125, 134, 140); + List linesMedium = Arrays.asList( + 21, 22, 23, 24, 25, 26, 29, 32, 44, 130, 135, 141, 154, 158, 159, 160, 166 + ); + List linesHigh = Arrays.asList( + 73, 77, 89, 101, 111, 116, 125, 134, 140, 161 + ); //List linesLow = Arrays.asList(57, 81, 121, 126, 136, 142); //Assertions @@ -83,9 +88,40 @@ public void detectCommandInjection() throws Exception { .build() ); + verify(reporter).doReportBug( + bugDefinition() + .bugType("COMMAND_INJECTION") + .inClass("MoreMethods").atLine(21) + .withPriority("High") + .build() + ); + + verify(reporter).doReportBug( + bugDefinition() + .bugType("COMMAND_INJECTION") + .inClass("MoreMethods").atLine(25) + .withPriority("High") + .build() + ); + + verify(reporter).doReportBug( + bugDefinition() + .bugType("COMMAND_INJECTION") + .inClass("SubClass").atLine(9) + .withPriority("High") + .build() + ); + verify(reporter).doReportBug( + bugDefinition() + .bugType("COMMAND_INJECTION") + .inClass("SubClass").atLine(10) + .withPriority("High") + .build() + ); + verify(reporter, times(linesMedium.size())).doReportBug( bugDefinition().bugType("COMMAND_INJECTION").withPriority("Medium").build()); - verify(reporter, times(linesHigh.size() + 1)).doReportBug( + verify(reporter, times(linesHigh.size() + 5)).doReportBug( bugDefinition().bugType("COMMAND_INJECTION").withPriority("High").build()); verify(reporter, never()).doReportBug( bugDefinition().bugType("COMMAND_INJECTION").withPriority("Low").build()); diff --git a/plugin/src/test/java/testcode/command/CommandInjection.java b/plugin/src/test/java/testcode/command/CommandInjection.java index f62fe0558..56c482a70 100644 --- a/plugin/src/test/java/testcode/command/CommandInjection.java +++ b/plugin/src/test/java/testcode/command/CommandInjection.java @@ -6,15 +6,16 @@ import java.io.IOException; import java.io.InputStreamReader; import java.util.Arrays; +import java.util.HashSet; import java.util.LinkedList; import java.util.List; +import java.util.ListIterator; public abstract class CommandInjection { public static void main(String[] args) throws IOException { String input = args.length > 0 ? args[0] : ";cat /etc/passwd"; List cmd = Arrays.asList("ls", "-l", input); - //Runtime exec() Runtime r = Runtime.getRuntime(); r.exec("ls -l " + input); @@ -23,7 +24,6 @@ public static void main(String[] args) throws IOException { r.exec(cmd.toArray(new String[cmd.size()])); r.exec(cmd.toArray(new String[cmd.size()]), null); r.exec(cmd.toArray(new String[cmd.size()]), null, null); - //ProcessBuilder new ProcessBuilder() .command("ls", "-l", input) @@ -154,6 +154,18 @@ public void testUnknown() throws IOException { Runtime.getRuntime().exec(sb.toString()); } + public void testListIterator() throws IOException { + Runtime.getRuntime().exec(transferListIteratorIndirect("safe")); + Runtime.getRuntime().exec(transferListIteratorIndirect(taintSourceDouble())); + Runtime.getRuntime().exec(transferThroughListIterator("safe")); + Runtime.getRuntime().exec(transferThroughListIterator(taintSourceDouble())); + } + + public void unknownSubmethod(String unknown) throws IOException { + Runtime.getRuntime().exec(new MoreMethods().safeParentparametricChild(unknown)); + Runtime.getRuntime().exec(new SubClass().safeParentparametricChild(unknown)); + } + private String transferThroughArray(String in) { String[] strings = new String[3]; strings[0] = "safe1"; @@ -178,12 +190,27 @@ private String transferThroughList(String in, int index) { + list.remove(index) + list.removeFirst() + list.removeLast() + list.set(index, "safe") + list.toString(); } + + private String transferThroughListIterator(String str) { + List list = new LinkedList(); + ListIterator listIterator = list.listIterator(); + listIterator.add(str); + return listIterator.next(); + } + + private String transferListIteratorIndirect(String str) { + List list = new LinkedList(); + // not able to transfer this, set as UNKNOWN even if str is SAFE + ListIterator listIterator = list.listIterator(); + listIterator.add(str); + return list.get(0); + } public String parametricUnknownSource(String str) { return str + new Object().toString() + "xx"; } - public String taintSource(String param) throws Exception { + public String taintSource(String param) throws IOException { File file = new File("C:\\data.txt"); FileInputStream streamFileInput; InputStreamReader readerInputStream; @@ -194,7 +221,7 @@ public String taintSource(String param) throws Exception { return param + readerBuffered.readLine(); } - public String taintSourceDouble() throws Exception { + public String taintSourceDouble() throws IOException { return taintSource("safe, but result will be tainted") + safeSource(1); } @@ -209,10 +236,32 @@ public String safeSource(long number) { public String combine(String x, String y) { StringBuilder sb = new StringBuilder("safe"); sb.append((Object) x); - return sb.toString() + y.concat("aaa"); + HashSet set = new HashSet(); + set.add("ooo"); + set.add(sb.append("x").append("y").toString().toLowerCase()); + for (String str : set) { + if (str.equals(y.toLowerCase())) { + return str; + } + } + return new StringBuilder(y).toString().trim() + "a".concat("aaa"); } public void call() throws IOException { MoreMethods.sink(System.getenv("")); } + + public void callInterface(InterfaceWithSink obj1) throws IOException { + InterfaceWithSink obj2 = getNewMoreMethods(); + if (obj2.hashCode() % 2 == 0) { + System.out.println(obj2.toString()); + } // just to confuse the analysis a bit + unknown(new StringBuilder().append(obj2)); + obj2.sink2(System.getenv("")); + obj1.sink2(System.getenv("")); // should not be reported + } + + public InterfaceWithSink getNewMoreMethods() { + return new MoreMethods(); + } } diff --git a/plugin/src/test/java/testcode/command/InterfaceWithSink.java b/plugin/src/test/java/testcode/command/InterfaceWithSink.java new file mode 100644 index 000000000..c4d8b9ff4 --- /dev/null +++ b/plugin/src/test/java/testcode/command/InterfaceWithSink.java @@ -0,0 +1,8 @@ +package testcode.command; + +import java.io.IOException; + +public interface InterfaceWithSink { + + void sink2(String param) throws IOException; +} diff --git a/plugin/src/test/java/testcode/command/MoreMethods.java b/plugin/src/test/java/testcode/command/MoreMethods.java index b3e7b2316..ee29f311b 100644 --- a/plugin/src/test/java/testcode/command/MoreMethods.java +++ b/plugin/src/test/java/testcode/command/MoreMethods.java @@ -2,7 +2,7 @@ import java.io.IOException; -public class MoreMethods { +public class MoreMethods implements InterfaceWithSink { public static String tainted() { return System.getenv("var"); @@ -15,4 +15,21 @@ public String safe() { public static void sink(String param) throws IOException { Runtime.getRuntime().exec(param); } + + @Override + public void sink2(String param) throws IOException { + Runtime.getRuntime().exec(param); + } + + public void sink3(String param) throws IOException { + Runtime.getRuntime().exec(param); + } + + public static String tainted2() { + return System.getenv("var2"); + } + + public String safeParentparametricChild(String param) { + return "safe parent"; + } } diff --git a/plugin/src/test/java/testcode/command/SubClass.java b/plugin/src/test/java/testcode/command/SubClass.java new file mode 100644 index 000000000..be87c89cb --- /dev/null +++ b/plugin/src/test/java/testcode/command/SubClass.java @@ -0,0 +1,18 @@ +package testcode.command; + +import java.io.IOException; + +public class SubClass extends MoreMethods { + + public void method() throws IOException { + Runtime.getRuntime().exec(safe()); + Runtime.getRuntime().exec(tainted()); + Runtime.getRuntime().exec(tainted2()); + sink3(System.getenv("")); + } + + @Override + public String safeParentparametricChild(String param) { + return param; + } +}