Skip to content

Commit 5756eda

Browse files
StevenArztStevenArzt
authored
Merge pull request #621 from timll/develop
Improve aliasing problem
2 parents dceadc6 + f108b20 commit 5756eda

File tree

4 files changed

+187
-64
lines changed

4 files changed

+187
-64
lines changed

‎soot-infoflow/src/soot/jimple/infoflow/problems/AliasProblem.java‎

Lines changed: 48 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -120,21 +120,6 @@ private Set<Abstraction> computeAliases(final DefinitionStmt defStmt, Value left
120120

121121
final Set<Abstraction> res = new MutableTwoElementSet<Abstraction>();
122122

123-
// Check whether the left side of the assignment matches our
124-
// current taint abstraction
125-
final boolean leftSideMatches = Aliasing.baseMatches(leftValue, source);
126-
if (!leftSideMatches)
127-
res.add(source);
128-
else {
129-
// The left side is overwritten completely
130-
131-
// If we have an assignment to the base local of the current
132-
// taint, all taint propagations must be below that point,
133-
// so this is the right point to turn around.
134-
for (Unit u : interproceduralCFG().getPredsOf(defStmt))
135-
manager.getMainSolver().processEdge(new PathEdge<Unit, Abstraction>(d1, u, source));
136-
}
137-
138123
// We only handle assignments and identity statements
139124
if (defStmt instanceof IdentityStmt) {
140125
res.add(source);
@@ -143,6 +128,12 @@ private Set<Abstraction> computeAliases(final DefinitionStmt defStmt, Value left
143128
if (!(defStmt instanceof AssignStmt))
144129
return res;
145130

131+
// Check whether the left side of the assignment matches our
132+
// current taint abstraction
133+
final boolean leftSideMatches = Aliasing.baseMatches(leftValue, source);
134+
if (!leftSideMatches)
135+
res.add(source);
136+
146137
// Get the right side of the assignment
147138
final Value rightValue = BaseSelector.selectBase(defStmt.getRightOp(), false);
148139

@@ -240,21 +231,12 @@ else if (defStmt.getRightOp() instanceof LengthExpr) {
240231
newLeftAbs = checkAbstraction(source.deriveNewAbstraction(ap, defStmt));
241232
}
242233

243-
if (newLeftAbs != null) {
244-
// If we ran into a new abstraction that points to a
245-
// primitive value, we can remove it
246-
if (newLeftAbs.getAccessPath().getLastFieldType() instanceof PrimType)
247-
return res;
248-
249-
if (!newLeftAbs.getAccessPath().equals(source.getAccessPath())) {
250-
// Propagate the new alias upwards
251-
res.add(newLeftAbs);
252-
253-
// Inject the new alias into the forward solver
254-
for (Unit u : interproceduralCFG().getPredsOf(defStmt))
255-
manager.getMainSolver()
256-
.processEdge(new PathEdge<Unit, Abstraction>(d1, u, newLeftAbs));
257-
}
234+
if (newLeftAbs != null && !newLeftAbs.getAccessPath().equals(source.getAccessPath())) {
235+
// Only inject the new alias into the forward solver but never propagate it upwards
236+
// because the alias was created at this program point and won't be valid above.
237+
for (Unit u : interproceduralCFG().getPredsOf(defStmt))
238+
manager.getMainSolver()
239+
.processEdge(new PathEdge<Unit, Abstraction>(d1, u, newLeftAbs));
258240
}
259241
}
260242

@@ -734,6 +716,42 @@ public Set<Abstraction> computeTargets(Abstraction source, Abstraction d1,
734716
if (abs != null) {
735717
res.add(abs);
736718
registerActivationCallSite(callSite, callee, abs);
719+
720+
// Check whether the call site created an alias by having two equal
721+
// arguments, e.g. caller(o, o);. If yes, inject the other parameter
722+
// back into the callee.
723+
for (int argIndex = 0; !isReflectiveCallSite && argIndex < ie.getArgCount(); argIndex++) {
724+
if (i != argIndex && originalCallArg == ie.getArg(argIndex)) {
725+
AccessPath aliasAp = manager.getAccessPathFactory().copyWithNewValue(
726+
source.getAccessPath(), paramLocals[argIndex],
727+
source.getAccessPath().getBaseType(),
728+
false);
729+
Abstraction aliasAbs = checkAbstraction(
730+
source.deriveNewAbstraction(aliasAp, (Stmt) exitStmt));
731+
732+
manager.getMainSolver()
733+
.processEdge(new PathEdge<>(d1, exitStmt, aliasAbs));
734+
}
735+
}
736+
737+
// A foo(A a) {
738+
// return a;
739+
// }
740+
// A b = foo(a);
741+
// An alias is created using the returned value. If no assignment
742+
// happen inside the method, also no handover is triggered. Thus,
743+
// for this special case, we hand over the current taint and let the
744+
// forward analysis find out whether the return value actually created
745+
// an alias or not.
746+
for (Unit u : manager.getICFG().getStartPointsOf(callee)) {
747+
if (!(u instanceof ReturnStmt))
748+
continue;
749+
750+
if (paramLocals[i] == ((ReturnStmt) u).getOp()) {
751+
manager.getMainSolver().processEdge(new PathEdge<>(d1, exitStmt, source));
752+
break;
753+
}
754+
}
737755
}
738756
}
739757
}

‎soot-infoflow/src/soot/jimple/infoflow/problems/BackwardsAliasProblem.java‎

Lines changed: 30 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -15,22 +15,7 @@
1515
import soot.Type;
1616
import soot.Unit;
1717
import soot.Value;
18-
import soot.jimple.ArrayRef;
19-
import soot.jimple.AssignStmt;
20-
import soot.jimple.BinopExpr;
21-
import soot.jimple.CastExpr;
22-
import soot.jimple.DefinitionStmt;
23-
import soot.jimple.FieldRef;
24-
import soot.jimple.IdentityStmt;
25-
import soot.jimple.InstanceFieldRef;
26-
import soot.jimple.InstanceInvokeExpr;
27-
import soot.jimple.InstanceOfExpr;
28-
import soot.jimple.InvokeExpr;
29-
import soot.jimple.NewArrayExpr;
30-
import soot.jimple.ReturnStmt;
31-
import soot.jimple.StaticFieldRef;
32-
import soot.jimple.Stmt;
33-
import soot.jimple.UnopExpr;
18+
import soot.jimple.*;
3419
import soot.jimple.infoflow.InfoflowConfiguration;
3520
import soot.jimple.infoflow.InfoflowManager;
3621
import soot.jimple.infoflow.aliasing.Aliasing;
@@ -135,42 +120,57 @@ private Set<Abstraction> computeAliases(final DefinitionStmt defStmt, Abstractio
135120

136121
AccessPath ap = source.getAccessPath();
137122
Value sourceBase = ap.getPlainValue();
123+
Type rightType = rightOp.getType();
138124
boolean handoverLeftValue = false;
125+
boolean cutSubfield = false;
139126
boolean leftSideOverwritten = false;
140127
if (leftOp instanceof StaticFieldRef) {
141128
if (manager.getConfig()
142129
.getStaticFieldTrackingMode() != InfoflowConfiguration.StaticFieldTrackingMode.None
143130
&& ap.firstFieldMatches(((StaticFieldRef) leftOp).getField())) {
144131
handoverLeftValue = true;
132+
cutSubfield = true;
145133
}
146134
} else if (leftOp instanceof InstanceFieldRef) {
147135
InstanceFieldRef instRef = (InstanceFieldRef) leftOp;
148136

149137
// base matches
150138
if (instRef.getBase() == sourceBase) {
151-
// field matches
152-
if (ap.firstFieldMatches(instRef.getField())) {
153-
handoverLeftValue = true;
154-
}
155-
// whole object matches
156-
else if (ap.getTaintSubFields() && ap.getFragmentCount() == 0) {
157-
handoverLeftValue = true;
158-
}
159-
// due to cut down access path we can not know better
160-
else if (source.dependsOnCutAP() || isCircularType(leftVal)) {
139+
AccessPath mappedAp = Aliasing.getReferencedAPBase(ap,
140+
new SootField[] { instRef.getField() }, manager);
141+
if (mappedAp != null) {
161142
handoverLeftValue = true;
143+
cutSubfield = true;
144+
if (!mappedAp.equals(ap))
145+
ap = mappedAp;
162146
}
163147
}
164148
} else if (leftVal == sourceBase) {
165149
// Either the alias is overwritten here or a write to an array element
166-
handoverLeftValue = leftOp instanceof ArrayRef;
150+
handoverLeftValue = leftOp instanceof ArrayRef
151+
&& ap.getArrayTaintType() != AccessPath.ArrayTaintType.Length;
167152
leftSideOverwritten = !handoverLeftValue;
168153
}
169154

170155
if (handoverLeftValue) {
171-
// We found a missed path upwards
172-
// inject same stmt in infoflow solver
173-
handOver(d1, srcUnit, source);
156+
Abstraction newAbs = null;
157+
if (rightVal instanceof Constant) {
158+
if (manager.getConfig().getImplicitFlowMode().trackControlFlowDependencies()) {
159+
newAbs = source.deriveConditionalUpdate(assignStmt);
160+
for (Unit pred : manager.getICFG().getPredsOf(srcUnit))
161+
handOver(d1, pred, newAbs);
162+
}
163+
} else {
164+
AccessPath newAp = manager.getAccessPathFactory().copyWithNewValue(ap, rightOp, rightType, cutSubfield);
165+
newAbs = source.deriveNewAbstraction(newAp, assignStmt);
166+
}
167+
168+
if (newAbs != null && !newAbs.equals(source)) {
169+
// We found a missed path upwards
170+
// inject same stmt in infoflow solver
171+
for (Unit pred : manager.getICFG().getPredsOf(srcUnit))
172+
handOver(d1, pred, newAbs);
173+
}
174174
}
175175

176176
if (leftSideOverwritten)

‎soot-infoflow/test/soot/jimple/infoflow/test/HeapTestCode.java‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -793,6 +793,24 @@ public void singleAliasTest() {
793793
cm.publish(b.b);
794794
}
795795

796+
public void negativeSingleAliasTest() {
797+
A a = new A();
798+
A b = fakeAlias(a);
799+
a.b = TelephonyManager.getDeviceId();
800+
ConnectionManager cm = new ConnectionManager();
801+
cm.publish(b.b);
802+
}
803+
804+
public A doNotFold() {
805+
A a = new A();
806+
System.out.println("XXX");
807+
return a;
808+
}
809+
810+
private A fakeAlias(A a) {
811+
return doNotFold();
812+
}
813+
796814
private int intData;
797815

798816
private void setIntData() {
@@ -1611,4 +1629,43 @@ public void activationStatementTest1() {
16111629
cm.publish(specialName);
16121630
}
16131631

1632+
public void callSiteCreatesAlias() {
1633+
String tainted = TelephonyManager.getDeviceId();
1634+
1635+
Book book1 = new Book();
1636+
leakingCallee(tainted, new Book(), book1);
1637+
leakingCallee(tainted, book1, book1);
1638+
}
1639+
1640+
void leakingCallee(String tainted, Book book1, Book book2) {
1641+
book1.name = tainted;
1642+
ConnectionManager cm = new ConnectionManager();
1643+
cm.publish(book2.name);
1644+
}
1645+
1646+
public Book alias;
1647+
public void lhsNotUpwardsInAliasFlow() {
1648+
alias = new Book();
1649+
1650+
Book book = new Book();
1651+
Book alias2 = alias;
1652+
alias = book; // alias only aliases book downwards from this program point
1653+
book.name = TelephonyManager.getDeviceId();
1654+
ConnectionManager cm = new ConnectionManager();
1655+
cm.publish(alias2.name);
1656+
}
1657+
1658+
public void identityStmtIsNotAGoodHandoverPoint() {
1659+
Book book = new Book();
1660+
// No need to propagate book forward again
1661+
callee(book);
1662+
book.name = TelephonyManager.getDeviceId();
1663+
1664+
ConnectionManager cm = new ConnectionManager();
1665+
cm.publish(book.name);
1666+
}
1667+
1668+
void callee(Book b) {
1669+
System.out.println(b);
1670+
}
16141671
}

‎soot-infoflow/test/soot/jimple/infoflow/test/junit/HeapTests.java‎

Lines changed: 52 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,7 @@
2020
import org.junit.Ignore;
2121
import org.junit.Test;
2222

23-
import soot.RefType;
24-
import soot.Scene;
25-
import soot.SootField;
26-
import soot.SootMethod;
23+
import soot.*;
2724
import soot.jimple.AssignStmt;
2825
import soot.jimple.DefinitionStmt;
2926
import soot.jimple.InstanceInvokeExpr;
@@ -38,6 +35,7 @@
3835
import soot.jimple.infoflow.data.SootMethodAndClass;
3936
import soot.jimple.infoflow.entryPointCreators.DefaultEntryPointCreator;
4037
import soot.jimple.infoflow.entryPointCreators.SequentialEntryPointCreator;
38+
import soot.jimple.infoflow.handlers.TaintPropagationHandler;
4139
import soot.jimple.infoflow.results.InfoflowResults;
4240
import soot.jimple.infoflow.sourcesSinks.definitions.MethodSourceSinkDefinition;
4341
import soot.jimple.infoflow.sourcesSinks.manager.ISourceSinkManager;
@@ -674,6 +672,18 @@ public void singleAliasTest() {
674672
checkInfoflow(infoflow, 1);
675673
}
676674

675+
@Test(timeout = 300000)
676+
public void negativeSingleAliasTest() {
677+
IInfoflow infoflow = initInfoflow();
678+
infoflow.getConfig().setInspectSources(false);
679+
infoflow.getConfig().setInspectSinks(false);
680+
681+
List<String> epoints = new ArrayList<String>();
682+
epoints.add("<soot.jimple.infoflow.test.HeapTestCode: void negativeSingleAliasTest()>");
683+
infoflow.computeInfoflow(appPath, libPath, epoints, sources, sinks);
684+
negativeCheckInfoflow(infoflow);
685+
}
686+
677687
@Test(timeout = 300000)
678688
public void intAliasTest() {
679689
IInfoflow infoflow = initInfoflow();
@@ -1269,4 +1279,42 @@ public void activationStatementTest1() {
12691279
negativeCheckInfoflow(infoflow);
12701280
}
12711281

1282+
1283+
@Test(timeout = 300000)
1284+
public void callSiteCreatesAlias() {
1285+
IInfoflow infoflow = initInfoflow();
1286+
List<String> epoints = new ArrayList<String>();
1287+
epoints.add("<soot.jimple.infoflow.test.HeapTestCode: void callSiteCreatesAlias()>");
1288+
infoflow.computeInfoflow(appPath, libPath, epoints, sources, sinks);
1289+
checkInfoflow(infoflow, 1);
1290+
}
1291+
1292+
@Test(timeout = 300000)
1293+
public void lhsNotUpwardsInAliasFlow() {
1294+
IInfoflow infoflow = initInfoflow();
1295+
List<String> epoints = new ArrayList<String>();
1296+
epoints.add("<soot.jimple.infoflow.test.HeapTestCode: void lhsNotUpwardsInAliasFlow()>");
1297+
infoflow.computeInfoflow(appPath, libPath, epoints, sources, sinks);
1298+
negativeCheckInfoflow(infoflow);
1299+
}
1300+
1301+
@Test(timeout = 300000)
1302+
public void identityStmtIsNotAGoodHandoverPoint() {
1303+
IInfoflow infoflow = initInfoflow();
1304+
List<String> epoints = new ArrayList<String>();
1305+
infoflow.setTaintPropagationHandler(new TaintPropagationHandler() {
1306+
@Override
1307+
public void notifyFlowIn(Unit stmt, Abstraction taint, InfoflowManager manager, FlowFunctionType type) {
1308+
Assert.assertTrue(taint.isAbstractionActive());
1309+
}
1310+
1311+
@Override
1312+
public Set<Abstraction> notifyFlowOut(Unit stmt, Abstraction d1, Abstraction incoming, Set<Abstraction> outgoing, InfoflowManager manager, FlowFunctionType type) {
1313+
return outgoing;
1314+
}
1315+
});
1316+
epoints.add("<soot.jimple.infoflow.test.HeapTestCode: void identityStmtIsNotAGoodHandoverPoint()>");
1317+
infoflow.computeInfoflow(appPath, libPath, epoints, sources, sinks);
1318+
checkInfoflow(infoflow, 1);
1319+
}
12721320
}

0 commit comments

Comments
 (0)