-
Notifications
You must be signed in to change notification settings - Fork 6
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add an example codemod that fixes missing annotations (#12)
It is a common use case that people want to add missing annotations to methods (e.g., authN/authZ). This changes adds an example codemod which does this.
- Loading branch information
Showing
7 changed files
with
229 additions
and
1 deletion.
There are no files selected for viewing
56 changes: 56 additions & 0 deletions
56
app/src/main/java/io/codemodder/sample/AddDecoratorCodemod.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,56 @@ | ||
package io.codemodder.sample; | ||
|
||
import static io.codemodder.ast.ASTTransforms.addImportIfMissing; | ||
|
||
import com.contrastsecurity.sarif.Result; | ||
import com.github.javaparser.ast.CompilationUnit; | ||
import com.github.javaparser.ast.body.ClassOrInterfaceDeclaration; | ||
import com.github.javaparser.ast.expr.AnnotationExpr; | ||
import io.codemodder.*; | ||
import io.codemodder.javaparser.ChangesResult; | ||
import io.codemodder.providers.sarif.semgrep.SemgrepScan; | ||
import java.util.Optional; | ||
import javax.inject.Inject; | ||
|
||
/** Adds a decorator @DoesDeserialization for types that are serializable for historic reasons. This is a common pattern for many custom codemod cases where a missing annotation that might enforce authentication or access control on a given type/method. */ | ||
@Codemod( | ||
id = "pixee:java/add-decorator", | ||
reviewGuidance = ReviewGuidance.MERGE_WITHOUT_REVIEW, | ||
importance = Importance.MEDIUM, | ||
executionPriority = CodemodExecutionPriority.LOW) | ||
public final class AddDecoratorCodemod | ||
extends SarifPluginJavaParserChanger<ClassOrInterfaceDeclaration> { | ||
|
||
private static final String DETECTION_RULE = | ||
""" | ||
rules: | ||
- id: add-decorator | ||
pattern: public class $CLAZZ implements Serializable | ||
"""; | ||
|
||
@Inject | ||
public AddDecoratorCodemod(@SemgrepScan(yaml = DETECTION_RULE) final RuleSarif sarif) { | ||
|
||
// usually we don't have to specify a particular RegionNodeMatcher, but JavaParser returns the range including everything from beginning of class to end of class, so we relax the matching logic to just match the given line so it lines up with what Semgrep reports | ||
super( | ||
sarif, | ||
ClassOrInterfaceDeclaration.class, | ||
RegionNodeMatcher.MATCHES_LINE); | ||
} | ||
|
||
@Override | ||
public ChangesResult onResultFound( | ||
final CodemodInvocationContext context, | ||
final CompilationUnit cu, | ||
final ClassOrInterfaceDeclaration classOrInterfaceDeclaration, | ||
final Result result) { | ||
Optional<AnnotationExpr> dummyAnnotation = | ||
classOrInterfaceDeclaration.getAnnotationByName("DoesSerialization"); | ||
if (dummyAnnotation.isEmpty()) { | ||
classOrInterfaceDeclaration.addAnnotation("DoesSerialization"); | ||
addImportIfMissing(cu, "com.acme.DoesSerialization"); | ||
return ChangesResult.changesApplied; | ||
} | ||
return ChangesResult.noChanges; | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1 change: 1 addition & 0 deletions
1
app/src/main/resources/io/codemodder/sample/AddDecoratorCodemod/description.md
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
This changes adds a decorator `@DoesSerialization`. to all `Serializable` types. For historical reasons, our static analysis checks expected this annotation to be everywhere we have serializable types, so this cleans up common developer toil of adding it when its missing. |
7 changes: 7 additions & 0 deletions
7
app/src/main/resources/io/codemodder/sample/AddDecoratorCodemod/report.json
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
{ | ||
"summary": "Adds the @DoesSerialization annotation to serializable types", | ||
"change": "Adds the missing annotation", | ||
"references": [ | ||
"https://docs.oracle.com/javase/8/docs/api/java/io/Serializable.html" | ||
] | ||
} |
10 changes: 10 additions & 0 deletions
10
app/src/test/java/io/codemodder/sample/AddDecoratorCodemodTest.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
package io.codemodder.sample; | ||
|
||
import io.codemodder.testutils.CodemodTestMixin; | ||
import io.codemodder.testutils.Metadata; | ||
|
||
@Metadata( | ||
codemodType = AddDecoratorCodemod.class, | ||
testResourceDir = "add-decorator", | ||
dependencies = {}) | ||
final class AddDecoratorCodemodTest implements CodemodTestMixin { } |
78 changes: 78 additions & 0 deletions
78
app/src/test/resources/add-decorator/VulnerableTaskHolder.java.after
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,78 @@ | ||
package org.dummy.insecure.framework; | ||
|
||
import com.acme.DoesSerialization; | ||
import java.io.BufferedReader; | ||
import java.io.IOException; | ||
import java.io.InputStreamReader; | ||
import java.io.ObjectInputStream; | ||
import java.io.Serializable; | ||
import java.time.LocalDateTime; | ||
import lombok.extern.slf4j.Slf4j; | ||
|
||
@Slf4j | ||
@DoesSerialization | ||
// TODO move back to lesson | ||
public class VulnerableTaskHolder implements Serializable { | ||
|
||
private static final long serialVersionUID = 2; | ||
|
||
private String taskName; | ||
private String taskAction; | ||
private LocalDateTime requestedExecutionTime; | ||
|
||
public VulnerableTaskHolder(String taskName, String taskAction) { | ||
super(); | ||
this.taskName = taskName; | ||
this.taskAction = taskAction; | ||
this.requestedExecutionTime = LocalDateTime.now(); | ||
} | ||
|
||
@Override | ||
public String toString() { | ||
return "VulnerableTaskHolder [taskName=" | ||
+ taskName | ||
+ ", taskAction=" | ||
+ taskAction | ||
+ ", requestedExecutionTime=" | ||
+ requestedExecutionTime | ||
+ "]"; | ||
} | ||
|
||
/** | ||
* Execute a task when de-serializing a saved or received object. | ||
* | ||
* @author stupid develop | ||
*/ | ||
private void readObject(ObjectInputStream stream) throws Exception { | ||
// unserialize data so taskName and taskAction are available | ||
stream.defaultReadObject(); | ||
|
||
// do something with the data | ||
log.info("restoring task: {}", taskName); | ||
log.info("restoring time: {}", requestedExecutionTime); | ||
|
||
if (requestedExecutionTime != null | ||
&& (requestedExecutionTime.isBefore(LocalDateTime.now().minusMinutes(10)) | ||
|| requestedExecutionTime.isAfter(LocalDateTime.now()))) { | ||
// do nothing is the time is not within 10 minutes after the object has been created | ||
log.debug(this.toString()); | ||
throw new IllegalArgumentException("outdated"); | ||
} | ||
|
||
// condition is here to prevent you from destroying the goat altogether | ||
if ((taskAction.startsWith("sleep") || taskAction.startsWith("ping")) | ||
&& taskAction.length() < 22) { | ||
log.info("about to execute: {}", taskAction); | ||
try { | ||
Process p = Runtime.getRuntime().exec(taskAction); | ||
BufferedReader in = new BufferedReader(new InputStreamReader(p.getInputStream())); | ||
String line = null; | ||
while ((line = in.readLine()) != null) { | ||
log.info(line); | ||
} | ||
} catch (IOException e) { | ||
log.error("IO Exception", e); | ||
} | ||
} | ||
} | ||
} |
76 changes: 76 additions & 0 deletions
76
app/src/test/resources/add-decorator/VulnerableTaskHolder.java.before
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,76 @@ | ||
package org.dummy.insecure.framework; | ||
|
||
import java.io.BufferedReader; | ||
import java.io.IOException; | ||
import java.io.InputStreamReader; | ||
import java.io.ObjectInputStream; | ||
import java.io.Serializable; | ||
import java.time.LocalDateTime; | ||
import lombok.extern.slf4j.Slf4j; | ||
|
||
@Slf4j | ||
// TODO move back to lesson | ||
public class VulnerableTaskHolder implements Serializable { | ||
|
||
private static final long serialVersionUID = 2; | ||
|
||
private String taskName; | ||
private String taskAction; | ||
private LocalDateTime requestedExecutionTime; | ||
|
||
public VulnerableTaskHolder(String taskName, String taskAction) { | ||
super(); | ||
this.taskName = taskName; | ||
this.taskAction = taskAction; | ||
this.requestedExecutionTime = LocalDateTime.now(); | ||
} | ||
|
||
@Override | ||
public String toString() { | ||
return "VulnerableTaskHolder [taskName=" | ||
+ taskName | ||
+ ", taskAction=" | ||
+ taskAction | ||
+ ", requestedExecutionTime=" | ||
+ requestedExecutionTime | ||
+ "]"; | ||
} | ||
|
||
/** | ||
* Execute a task when de-serializing a saved or received object. | ||
* | ||
* @author stupid develop | ||
*/ | ||
private void readObject(ObjectInputStream stream) throws Exception { | ||
// unserialize data so taskName and taskAction are available | ||
stream.defaultReadObject(); | ||
|
||
// do something with the data | ||
log.info("restoring task: {}", taskName); | ||
log.info("restoring time: {}", requestedExecutionTime); | ||
|
||
if (requestedExecutionTime != null | ||
&& (requestedExecutionTime.isBefore(LocalDateTime.now().minusMinutes(10)) | ||
|| requestedExecutionTime.isAfter(LocalDateTime.now()))) { | ||
// do nothing is the time is not within 10 minutes after the object has been created | ||
log.debug(this.toString()); | ||
throw new IllegalArgumentException("outdated"); | ||
} | ||
|
||
// condition is here to prevent you from destroying the goat altogether | ||
if ((taskAction.startsWith("sleep") || taskAction.startsWith("ping")) | ||
&& taskAction.length() < 22) { | ||
log.info("about to execute: {}", taskAction); | ||
try { | ||
Process p = Runtime.getRuntime().exec(taskAction); | ||
BufferedReader in = new BufferedReader(new InputStreamReader(p.getInputStream())); | ||
String line = null; | ||
while ((line = in.readLine()) != null) { | ||
log.info(line); | ||
} | ||
} catch (IOException e) { | ||
log.error("IO Exception", e); | ||
} | ||
} | ||
} | ||
} |