Skip to content

Commit

Permalink
BuildStringPassthruGraph: proper handling of long/double arguments
Browse files Browse the repository at this point in the history
  • Loading branch information
@amaembo
amaembo committed Oct 6, 2015
1 parent dcfb934 commit 370808a
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 21 deletions.
46 changes: 27 additions & 19 deletions findbugs/src/java/edu/umd/cs/findbugs/detect/BuildStringPassthruGraph.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@

import org.apache.bcel.classfile.Code;
import org.apache.bcel.classfile.Method;
import org.apache.bcel.generic.Type;

import edu.umd.cs.findbugs.BugReporter;
import edu.umd.cs.findbugs.NonReportingDetector;
Expand Down Expand Up @@ -205,9 +206,7 @@ public Map<MethodDescriptor, int[]> getFileNameStringMethods() {

private int nArgs;

private int shift;

private boolean[] argEnabled;
private int[] argNums;

private List<MethodParameter>[] passedParameters;

Expand All @@ -218,30 +217,36 @@ public BuildStringPassthruGraph(BugReporter bugReporter) {
@SuppressWarnings("unchecked")
@Override
public void visitMethod(Method obj) {
argEnabled = null;
org.apache.bcel.generic.Type[] argumentTypes = obj.getArgumentTypes();
argNums = null;
Type[] argumentTypes = obj.getArgumentTypes();
if(argumentTypes.length == 0) {
return;
}
int lvNum = obj.isStatic() ? 0 : 1;
nArgs = argumentTypes.length;
int argCount = lvNum;
for(Type type : argumentTypes) {
argCount+=type.getSize();
}
for(int i=0; i<nArgs; i++) {
if(argumentTypes[i].getSignature().equals("Ljava/lang/String;")) {
if(argEnabled == null) {
argEnabled = new boolean[nArgs];
if(argNums == null) {
argNums = new int[argCount];
Arrays.fill(argNums, -1);
}
argEnabled[i] = true;
argNums[lvNum] = i;
}
lvNum+=argumentTypes[i].getSize();
}
if(argEnabled != null) {
shift = obj.isStatic() ? 0 : -1;
if(argNums != null) {
passedParameters = new List[nArgs];
}
super.visitMethod(obj);
}

@Override
public boolean shouldVisitCode(Code obj) {
return argEnabled != null;
return argNums != null;
}

@Override
Expand All @@ -261,10 +266,13 @@ public void visitAfter(Code obj) {
@Override
public void sawOpcode(int seen) {
if (isRegisterStore()) {
int param = getRegisterOperand() + shift;
if (param >= 0 && param < nArgs) {
argEnabled[param] = false;
passedParameters[param] = null;
int param = getRegisterOperand();
if (param < argNums.length) {
int argNum = argNums[param];
argNums[param] = -1;
if(argNum >= 0) {
passedParameters[argNum] = null;
}
}
}
switch (seen) {
Expand All @@ -276,11 +284,11 @@ public void sawOpcode(int seen) {
int callArgs = getNumberArguments(md.getSignature());
for (int i = 0; i < callArgs; i++) {
Item item = getStack().getStackItem(callArgs - 1 - i);
int param = item.getRegisterNumber() + shift;
if (param >= 0 && param < nArgs && argEnabled[param]) {
List<MethodParameter> list = passedParameters[param];
int param = item.getRegisterNumber();
if (param >= 0 && param < argNums.length && argNums[param] != -1) {
List<MethodParameter> list = passedParameters[argNums[param]];
if (list == null) {
passedParameters[param] = list = new ArrayList<>();
passedParameters[argNums[param]] = list = new ArrayList<>();
}
list.add(new MethodParameter(md, i));
}
Expand Down
7 changes: 5 additions & 2 deletions findbugs/src/java/edu/umd/cs/findbugs/detect/DumbMethodInvocations.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import edu.umd.cs.findbugs.ba.DataflowAnalysisException;
import edu.umd.cs.findbugs.ba.Location;
import edu.umd.cs.findbugs.ba.MethodUnprofitableException;
import edu.umd.cs.findbugs.ba.SignatureParser;
import edu.umd.cs.findbugs.ba.constant.Constant;
import edu.umd.cs.findbugs.ba.constant.ConstantDataflow;
import edu.umd.cs.findbugs.ba.constant.ConstantFrame;
Expand Down Expand Up @@ -95,6 +96,8 @@ private void analyzeMethod(ClassContext classContext, Method method) throws CFGB
}
InvokeInstruction iins = (InvokeInstruction) ins;

SignatureParser parser = new SignatureParser(iins.getSignature(cpg));

ConstantFrame frame = constantDataflow.getFactAtLocation(location);
if (!frame.isValid()) {
// This basic block is probably dead
Expand All @@ -104,7 +107,7 @@ private void analyzeMethod(ClassContext classContext, Method method) throws CFGB
MethodDescriptor md = new MethodDescriptor(iins, cpg);
if (allDatabasePasswordMethods.containsKey(md)) {
for(int paramNumber : allDatabasePasswordMethods.get(md)) {
Constant operandValue = frame.getStackValue(iins.getArgumentTypes(cpg).length-1-paramNumber);
Constant operandValue = frame.getArgument(iins, cpg, paramNumber, parser);
if (operandValue.isConstantString()) {
String password = operandValue.getConstantString();
if (password.length() == 0) {
Expand Down Expand Up @@ -134,7 +137,7 @@ private void analyzeMethod(ClassContext classContext, Method method) throws CFGB
} else if (allFileNameStringMethods.containsKey(md)) {

for(int paramNumber : allFileNameStringMethods.get(md)) {
Constant operandValue = frame.getStackValue(iins.getArgumentTypes(cpg).length-1-paramNumber);
Constant operandValue = frame.getArgument(iins, cpg, paramNumber, parser);
if (!operandValue.isConstantString()) {
continue;
}
Expand Down
14 changes: 14 additions & 0 deletions findbugsTestCases/src/java/sfBugsNew/Feature314.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,16 @@ public void testHardCodedPuzzlingOk() throws FileNotFoundException {
openFilePuzzling4("c:\\file.txt", "ok", "c:\\file.txt", "c:\\file.txt");
}

@ExpectWarning("DMI_HARDCODED_ABSOLUTE_FILENAME")
public void testHardCodedLong() throws FileNotFoundException {
openFilePuzzlingLong(1L, "c:\\file.txt", "ok", 0.0);
}

@NoWarning("DMI_HARDCODED_ABSOLUTE_FILENAME")
public void testHardCodedLongOk() throws FileNotFoundException {
openFilePuzzlingLong(1L, "ok", "c:\\file.txt", 0.0);
}

private FileOutputStream openFile(String name) throws FileNotFoundException {
return new FileOutputStream(name);
}
Expand All @@ -52,6 +62,10 @@ private FileOutputStream openFilePuzzling4(String arg1, String name, String arg2
return openFilePuzzling3(name, arg1, arg2, arg3);
}

private FileOutputStream openFilePuzzlingLong(long arg1, String name, String arg2, double arg3) throws FileNotFoundException {
return openFilePuzzling3(name, String.valueOf(arg1), arg2, String.valueOf(arg3));
}

@ExpectWarning("SQL_NONCONSTANT_STRING_PASSED_TO_EXECUTE")
public boolean test(Connection c, String code) throws SQLException {
return Sql.hasResult(c, "SELECT 1 FROM myTable WHERE code='"+code+"'");
Expand Down

0 comments on commit 370808a

Please sign in to comment.