Skip to content

For static method calls and field accesses do not look into superclasses #238

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

Merged
merged 11 commits into from
Nov 7, 2023
19 changes: 13 additions & 6 deletions src/main/java/de/thetaphi/forbiddenapis/ClassScanner.java
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,10 @@ static interface AncestorVisitor {
String visitAncestors(ClassMetadata cls, AncestorVisitor visitor, boolean visitSelf, boolean visitInterfacesFirst) {
if (visitSelf) {
final String result = visitor.visit(cls, cls.className, cls.isInterface, cls.isRuntimeClass);
if (result != null && result != AncestorVisitor.STOP) {
if (result == AncestorVisitor.STOP) {
return null;
}
if (result != null) {
return result;
}
}
Expand Down Expand Up @@ -379,7 +382,7 @@ public MethodVisitor visitMethod(final int access, final String name, final Stri
}
}

private String checkMethodAccess(String owner, final Method method) {
private String checkMethodAccess(String owner, final Method method, final boolean callIsVirtual) {
if (CLASS_CONSTRUCTOR_METHOD_NAME.equals(method.getName())) {
// we don't check for violations on class constructors
return null;
Expand Down Expand Up @@ -413,8 +416,10 @@ public String visit(ClassMetadata c, String origName, boolean isInterfaceOfAnces
if (!c.methods.contains(lookupMethod)) {
return null;
}
// is we have a virtual call, look into superclasses, otherwise stop:
final String notFoundRet = callIsVirtual ? null : AncestorVisitor.STOP;
if (previousInRuntime && c.isNonPortableRuntime) {
return null; // something inside the JVM is extending internal class/interface
return notFoundRet; // something inside the JVM is extending internal class/interface
}
String violation = forbiddenSignatures.checkMethod(c.className, lookupMethod);
if (violation != null) {
Expand All @@ -427,7 +432,7 @@ public String visit(ClassMetadata c, String origName, boolean isInterfaceOfAnces
return violation;
}
}
return null;
return notFoundRet;
}
}, true, false /* JVM spec says: interfaces after superclasses */);
}
Expand Down Expand Up @@ -492,7 +497,8 @@ private String checkHandle(Handle handle, boolean checkLambdaHandle) {
// so we can assign the called lambda with the same groupId like *this* method:
lambdas.put(m, currentGroupId);
}
return checkMethodAccess(handle.getOwner(), m);
final boolean callIsVirtual = (handle.getTag() == Opcodes.H_INVOKEVIRTUAL) || (handle.getTag() == Opcodes.H_INVOKEINTERFACE);
return checkMethodAccess(handle.getOwner(), m, callIsVirtual);
}
return null;
}
Expand Down Expand Up @@ -550,7 +556,8 @@ public AnnotationVisitor visitTryCatchAnnotation(int typeRef, TypePath typePath,

@Override
public void visitMethodInsn(int opcode, String owner, String name, String desc, boolean itf) {
reportMethodViolation(checkMethodAccess(owner, new Method(name, desc)), "method body");
final boolean callIsVirtual = (opcode == Opcodes.INVOKEVIRTUAL) || (opcode == Opcodes.INVOKEINTERFACE);
reportMethodViolation(checkMethodAccess(owner, new Method(name, desc), callIsVirtual), "method body");
}

@Override
Expand Down
Binary file added src/test/antunit/Java7StaticVsVirtual$X.class
Binary file not shown.
Binary file added src/test/antunit/Java7StaticVsVirtual$Y.class
Binary file not shown.
Binary file added src/test/antunit/Java7StaticVsVirtual$Z.class
Binary file not shown.
Binary file added src/test/antunit/Java7StaticVsVirtual.class
Binary file not shown.
65 changes: 65 additions & 0 deletions src/test/antunit/Java7StaticVsVirtual.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
/*
* (C) Copyright Uwe Schindler (Generics Policeman) and others.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

/* The binary class file is packaged together with the source distribution.
*/

import java.util.BitSet;

public class Java7StaticVsVirtual {
public static final long[] data = new long[] {1, 2, 3, 4};

public static void main(String[] args) {
X.valueOf(data).toString(); // Line 26 -- the only violation for static BitSet#valueOf(**)
Y.valueOf(data).toString(); // Line 27 -- should pass
Z.valueOf(data).toString(); // Line 28 -- should pass
Integer.toString(Y.a); // Line 29 -- violation (static field access)
Integer.toString(Z.a); // Line 30 -- should pass (hidden)
new X().get(0); // Line 31 -- violation (virtual methods detected regardles how they are called)
new Y().get(0); // Line 32 -- violation (virtual methods detected regardles how they are called)
new Z().get(0); // Line 33 -- violation (virtual methods detected regardles how they are called)
Integer.toString(new Y().b); // Line 34 -- violation
Integer.toString(new Z().b); // Line 35 -- should pass (hidden)
}

public static class X extends BitSet { }

public static class Y extends X {
public static int a;
public int b;

public static BitSet valueOf(long[] longs) {
return new BitSet();
}

@Override
public boolean get(int bit) {
return false;
}
}

public static class Z extends Y {
public static int a; // hides field in superclass
public int b; // hides field in superclass

public String goStatic() {
return valueOf(data).toString(); // Line 59 -- should pass
}
public boolean goVirtual() {
return get(0); // Line 62 -- violation (virtual methods detected regardles how they are called)
}
}
}
61 changes: 61 additions & 0 deletions src/test/antunit/TestStaticVsVirtual.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
<?xml version="1.0" encoding="UTF-8"?>
<!--
* (C) Copyright Uwe Schindler (Generics Policeman) and others.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
-->
<project xmlns:au="antlib:org.apache.ant.antunit">

<target name="testStatic">
<au:expectfailure expectedMessage="Check for forbidden API calls failed, see log">
<forbiddenapis>
<fileset file="Java7StaticVsVirtual*.class"/>
java.util.BitSet#valueOf(**) @ Forbidden static method
</forbiddenapis>
</au:expectfailure>
<au:assertLogContains level="error" text="Forbidden method invocation: java.util.BitSet#valueOf(**) [Forbidden static method]"/>
<au:assertLogContains level="error" text="(Java7StaticVsVirtual.java:26)"/>
<au:assertLogContains level="error" text=" 1 error(s)"/>
</target>

<target name="testVirtual">
<au:expectfailure expectedMessage="Check for forbidden API calls failed, see log">
<forbiddenapis>
<fileset file="Java7StaticVsVirtual*.class"/>
java.util.BitSet#get(int) @ Forbidden virtual method
</forbiddenapis>
</au:expectfailure>
<au:assertLogContains level="error" text="Forbidden method invocation: java.util.BitSet#get(int) [Forbidden virtual method]"/>
<au:assertLogContains level="error" text="(Java7StaticVsVirtual.java:31)"/>
<au:assertLogContains level="error" text="(Java7StaticVsVirtual.java:32)"/>
<au:assertLogContains level="error" text="(Java7StaticVsVirtual.java:33)"/>
<au:assertLogContains level="error" text="(Java7StaticVsVirtual.java:62)"/>
<au:assertLogContains level="error" text=" 4 error(s)"/>
</target>

<target name="testFields">
<au:expectfailure expectedMessage="Check for forbidden API calls failed, see log">
<forbiddenapis classpath=".">
<fileset file="Java7StaticVsVirtual*.class"/>
Java7StaticVsVirtual$Y#a @ Forbidden static field
Java7StaticVsVirtual$Y#b @ Forbidden instance field
</forbiddenapis>
</au:expectfailure>
<au:assertLogContains level="error" text="Forbidden field access: Java7StaticVsVirtual$Y#a [Forbidden static field]"/>
<au:assertLogContains level="error" text="(Java7StaticVsVirtual.java:29)"/>
<au:assertLogContains level="error" text="Forbidden field access: Java7StaticVsVirtual$Y#b [Forbidden instance field]"/>
<au:assertLogContains level="error" text="(Java7StaticVsVirtual.java:34)"/>
<au:assertLogContains level="error" text=" 2 error(s)"/>
</target>

</project>