Skip to content

Commit

Permalink
do not advise using doPrivileged above Java 17 target (spotbugs#3225)
Browse files Browse the repository at this point in the history
  • Loading branch information
JuditKnoll authored Dec 13, 2024
1 parent 461d901 commit 3bc80ea
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 20 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ Currently the versioning policy of this project follows [Semantic Versioning v2.
- Fixed `FI_FINALIZER_NULLS_FIELDS` FPs for functions called finalize() but not with the correct signature. ([#3207](https://github.com/spotbugs/spotbugs/issues/3207))
- Fixed an error in the detection of bridge methods causing analysis crashes ([#3208](https://github.com/spotbugs/spotbugs/issues/3208))
- Fixed detector `ThrowingExceptions` by removing false positive reports, such as synthetic methods (lambdas), methods which inherited their exception specifications and methods which call throwing methods ([#2040](https://github.com/spotbugs/spotbugs/issues/2040))
- Do not report `DP_DO_INSIDE_DO_PRIVILEGED`, `DP_CREATE_CLASSLOADER_INSIDE_DO_PRIVILEGED` and `USC_POTENTIAL_SECURITY_CHECK_BASED_ON_UNTRUSTED_SOURCE` in code targeting Java 17 and above, since it advises the usage of deprecated method ([#1515](https://github.com/spotbugs/spotbugs/issues/1515)).

### Cleanup
- Cleanup thread issue and regex issue in test-harness ([#3130](https://github.com/spotbugs/spotbugs/issues/3130))
Expand Down
45 changes: 29 additions & 16 deletions spotbugs/etc/messages.xml
Original file line number Diff line number Diff line change
Expand Up @@ -2582,33 +2582,42 @@ will need to be changed in order to compile it in later versions of Java.</p>
<ShortDescription>Method invoked that should be only be invoked inside a doPrivileged block</ShortDescription>
<LongDescription>Invocation of {2}, which should be invoked from within a doPrivileged block, in {1}</LongDescription>
<Details>
<![CDATA[
<p> This code invokes a method that requires a security permission check.
If this code will be granted security permissions, but might be invoked by code that does not
have security permissions, then the invocation needs to occur inside a doPrivileged block.</p>
]]>
<![CDATA[
<p> This code invokes a method that requires a security permission check.
If this code will be granted security permissions, but might be invoked by code that does not
have security permissions, then the invocation needs to occur inside a doPrivileged block.</p>
The <code>java.security.AccessController<\code> class, which contains the <code>doPrivileged</code> methods,
got deprecated in Java 17 (see <a href="https://openjdk.org/jeps/411">JEP 411</a>), and removed in Java 24 (see <a href="https://openjdk.org/jeps/486">JEP 486</a>).
For this reason, this bug isn't reported in classes targeted Java 17 and above.
]]>
</Details>
</BugPattern>
<BugPattern type="DP_DO_INSIDE_DO_PRIVILEDGED"> <!-- misspelled for backward compatibility -->
<ShortDescription>Method invoked that should be only be invoked inside a doPrivileged block</ShortDescription>
<LongDescription>Invocation of {2}, which should be invoked from within a doPrivileged block, in {1}</LongDescription>
<Details>
<![CDATA[
<p> This code invokes a method that requires a security permission check.
If this code will be granted security permissions, but might be invoked by code that does not
have security permissions, then the invocation needs to occur inside a doPrivileged block.</p>
]]>
<![CDATA[
<p> This code invokes a method that requires a security permission check.
If this code will be granted security permissions, but might be invoked by code that does not
have security permissions, then the invocation needs to occur inside a doPrivileged block.</p>
The <code>java.security.AccessController<\code> class, which contains the <code>doPrivileged</code> methods,
got deprecated in Java 17 (see <a href="https://openjdk.org/jeps/411">JEP 411</a>), and removed in Java 24 (see <a href="https://openjdk.org/jeps/486">JEP 486</a>).
For this reason, this bug isn't reported in classes targeted Java 17 and above.
]]>
</Details>
</BugPattern>
<BugPattern type="DP_CREATE_CLASSLOADER_INSIDE_DO_PRIVILEGED">
<ShortDescription>Classloaders should only be created inside doPrivileged block</ShortDescription>
<LongDescription>{1} creates a {2} classloader, which should be performed within a doPrivileged block</LongDescription>
<Details>
<![CDATA[
<p> This code creates a classloader, which needs permission if a security manage is installed.
If this code might be invoked by code that does not
have security permissions, then the classloader creation needs to occur inside a doPrivileged block.</p>
]]>
<![CDATA[
<p> This code creates a classloader, which needs permission if a security manage is installed.
If this code might be invoked by code that does not
have security permissions, then the classloader creation needs to occur inside a doPrivileged block.</p>
The <code>java.security.AccessController<\code> class, which contains the <code>doPrivileged</code> methods,
got deprecated in Java 17 (see <a href="https://openjdk.org/jeps/411">JEP 411</a>), and removed in Java 24 (see <a href="https://openjdk.org/jeps/486">JEP 486</a>).
For this reason, this bug isn't reported in classes targeted Java 17 and above.
]]>
</Details>
</BugPattern>
<BugPattern type="JCIP_FIELD_ISNT_FINAL_IN_IMMUTABLE_CLASS">
Expand Down Expand Up @@ -9068,7 +9077,11 @@ Using floating-point variables should not be used as loop counters, as they are
method behaves exactly as expected.
<p>
See SEI CERT rule <a href="https://wiki.sei.cmu.edu/confluence/display/java/SEC02-J.+Do+not+base+security+checks+on+untrusted+sources">SEC02-J. Do not base security checks on untrusted sources</a>.
</p>]]>
</p>
The <code>java.security.AccessController<\code> class, which contains the <code>doPrivileged</code> methods,
got deprecated in Java 17 (see <a href="https://openjdk.org/jeps/411">JEP 411</a>), and removed in Java 24 (see <a href="https://openjdk.org/jeps/486">JEP 486</a>).
For this reason, this bug isn't reported in classes targeted Java 17 and above.
]]>
</Details>
</BugPattern>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,17 +40,22 @@ public DoInsideDoPrivileged(BugReporter bugReporter) {
this.bugAccumulator = new BugAccumulator(bugReporter);
}

boolean isDoPrivileged = false;
private boolean isDoPrivileged = false;
private boolean isDoPrivilegedDeprecated = false;

@Override
public void visit(JavaClass obj) {
isDoPrivilegedDeprecated = obj.getMajor() >= Const.MAJOR_17;

isDoPrivileged = Subtypes2.instanceOf(getDottedClassName(), "java.security.PrivilegedAction")
|| Subtypes2.instanceOf(getDottedClassName(), "java.security.PrivilegedExceptionAction");
}

@Override
public void visit(Code obj) {
if (isDoPrivilegedDeprecated) {
return;
}
if (isDoPrivileged && "run".equals(getMethodName())) {
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ private static class CallPair {
private Stack<String> parameterNameStack = new Stack<>();

private LambdaInfo currentLambda = null;

private boolean isDoPrivilegedDeprecated = false;
private boolean isDoPrivileged = false;
private boolean isDoPrivilegedRun = false;
private boolean isLambdaCalledInDoPrivileged = false;
Expand All @@ -134,6 +134,7 @@ public FindPotentialSecurityCheckBasedOnUntrustedSource(BugReporter bugReporter)

@Override
public void visit(JavaClass obj) {
isDoPrivilegedDeprecated = obj.getMajor() >= Const.MAJOR_17;
nonFinalMethodsCalledOnParam.clear();
isDoPrivileged = Subtypes2.instanceOf(getDottedClassName(), "java.security.PrivilegedAction")
|| Subtypes2.instanceOf(getDottedClassName(), "java.security.PrivilegedExceptionAction");
Expand All @@ -148,8 +149,8 @@ public void visit(Method obj) {

@Override
public void visit(Code obj) {
if (!isDoPrivilegedRun && !isLambdaCalledInDoPrivileged &&
(!getThisClass().isPublic() || !getMethod().isPublic())) {
if (isDoPrivilegedDeprecated
|| (!isDoPrivilegedRun && !isLambdaCalledInDoPrivileged && (!getThisClass().isPublic() || !getMethod().isPublic()))) {
return;
}
super.visit(obj);
Expand Down

0 comments on commit 3bc80ea

Please sign in to comment.