New Sonar Codemod for Object Deserialization (#451)

\close #work

core-codemods/src/main/java/io/codemodder/codemods/DefaultCodemods.java

@@ -75,11 +75,12 @@ public static List<Class<? extends CodeChanger>> asList() {
         SimplifyRestControllerAnnotationsCodemod.class,
         SubstituteReplaceAllCodemod.class,
         SonarJNDIInjectionCodemod.class,
+        SonarObjectDeserializationCodemod.class,
         SonarRemoveUnthrowableExceptionCodemod.class,
-        SonarXXECodemod.class,
         SonarSQLInjectionCodemod.class,
-        SonarUnsafeReflectionRemediationCodemod.class,
         SonarSSRFCodemod.class,
+        SonarUnsafeReflectionRemediationCodemod.class,
+        SonarXXECodemod.class,
         SQLParameterizerCodemod.class,
         SSRFCodemod.class,
         StackTraceExposureCodemod.class,

core-codemods/src/main/java/io/codemodder/codemods/SonarObjectDeserializationCodemod.java

@@ -0,0 +1,58 @@
+package io.codemodder.codemods;
+
+import com.github.javaparser.ast.CompilationUnit;
+import io.codemodder.*;
+import io.codemodder.codetf.DetectorRule;
+import io.codemodder.providers.sonar.ProvidedSonarScan;
+import io.codemodder.providers.sonar.RuleIssue;
+import io.codemodder.providers.sonar.SonarRemediatingJavaParserChanger;
+import io.codemodder.remediation.GenericRemediationMetadata;
+import io.codemodder.remediation.javadeserialization.JavaDeserializationRemediator;
+import io.codemodder.sonar.model.Issue;
+import io.codemodder.sonar.model.SonarFinding;
+import java.util.List;
+import java.util.Objects;
+import javax.inject.Inject;
+
+/** Fixes Object Deserialization issues found by sonar rule javasecurity:S5135. */
+@Codemod(
+    id = "sonar:java/object-deserialization-s5135",
+    reviewGuidance = ReviewGuidance.MERGE_WITHOUT_REVIEW,
+    executionPriority = CodemodExecutionPriority.HIGH,
+    importance = Importance.HIGH)
+public final class SonarObjectDeserializationCodemod extends SonarRemediatingJavaParserChanger {
+
+  private final JavaDeserializationRemediator remediator;
+  private final RuleIssue issues;
+
+  @Inject
+  public SonarObjectDeserializationCodemod(
+      @ProvidedSonarScan(ruleId = "javasecurity:S5135") final RuleIssue issues) {
+    super(GenericRemediationMetadata.DESERIALIZATION.reporter(), issues);
+    this.issues = Objects.requireNonNull(issues);
+    this.remediator = JavaDeserializationRemediator.DEFAULT;
+  }
+
+  @Override
+  public DetectorRule detectorRule() {
+    return new DetectorRule(
+        "javasecurity:S5135",
+        "Deserialization should not be vulnerable to injection attacks",
+        "https://rules.sonarsource.com/java/RSPEC-5135/");
+  }
+
+  @Override
+  public CodemodFileScanningResult visit(
+      final CodemodInvocationContext context, final CompilationUnit cu) {
+    List<Issue> issuesForFile = issues.getResultsByPath(context.path());
+    return remediator.remediateAll(
+        cu,
+        context.path().toString(),
+        detectorRule(),
+        issuesForFile,
+        SonarFinding::getKey,
+        i -> i.getTextRange() != null ? i.getTextRange().getStartLine() : i.getLine(),
+        i -> i.getTextRange() != null ? i.getTextRange().getEndLine() : null,
+        i -> i.getTextRange() != null ? i.getTextRange().getStartOffset() : null);
+  }
+}

core-codemods/src/test/java/io/codemodder/codemods/SonarObjectDeserializationCodemodTest.java

@@ -0,0 +1,14 @@
+package io.codemodder.codemods;
+
+import io.codemodder.DependencyGAV;
+import io.codemodder.testutils.CodemodTestMixin;
+import io.codemodder.testutils.Metadata;
+
+@Metadata(
+    codemodType = SonarObjectDeserializationCodemod.class,
+    testResourceDir = "sonar-object-deserialization-s5135",
+    renameTestFile =
+        "src/main/java/org/owasp/webgoat/lessons/deserialization/InsecureDeserializationTask.java",
+    expectingFixesAtLines = {60},
+    dependencies = DependencyGAV.JAVA_SECURITY_TOOLKIT_GAV)
+final class SonarObjectDeserializationCodemodTest implements CodemodTestMixin {}

core-codemods/src/test/java/io/codemodder/codemods/SonarSSRFCodemodTest.java

@@ -15,5 +15,5 @@ final class SonarSSRFCodemodTest {
           "src/main/java/org/owasp/webgoat/lessons/passwordreset/ResetLinkAssignmentForgotPassword.java",
       expectingFixesAtLines = {104},
       dependencies = DependencyGAV.JAVA_SECURITY_TOOLKIT_GAV)
-  class RestTemplateTest implements CodemodTestMixin {}
+  final class RestTemplateTest implements CodemodTestMixin {}
 }

core-codemods/src/test/resources/sonar-object-deserialization-s5135/InsecureDeserializationTask.java.after

@@ -0,0 +1,86 @@
+/*
+ * This file is part of WebGoat, an Open Web Application Security Project utility. For details, please see http://www.owasp.org/
+ *
+ * Copyright (c) 2002 - 2019 Bruce Mayhew
+ *
+ * This program is free software; you can redistribute it and/or modify it under the terms of the
+ * GNU General Public License as published by the Free Software Foundation; either version 2 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without
+ * even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along with this program; if
+ * not, write to the Free Software Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA
+ * 02111-1307, USA.
+ *
+ * Getting Source ==============
+ *
+ * Source for this application is maintained at https://github.com/WebGoat/WebGoat, a repository for free software projects.
+ */
+
+package org.owasp.webgoat.lessons.deserialization;
+
+import static io.github.pixee.security.ObjectInputFilters.createSafeObjectInputStream;
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InvalidClassException;
+import java.io.ObjectInputStream;
+import java.util.Base64;
+import org.dummy.insecure.framework.VulnerableTaskHolder;
+import org.owasp.webgoat.container.assignments.AssignmentEndpoint;
+import org.owasp.webgoat.container.assignments.AssignmentHints;
+import org.owasp.webgoat.container.assignments.AttackResult;
+import org.springframework.web.bind.annotation.PostMapping;
+import org.springframework.web.bind.annotation.RequestParam;
+import org.springframework.web.bind.annotation.ResponseBody;
+import org.springframework.web.bind.annotation.RestController;
+
+@RestController
+@AssignmentHints({
+  "insecure-deserialization.hints.1",
+  "insecure-deserialization.hints.2",
+  "insecure-deserialization.hints.3"
+})
+public class InsecureDeserializationTask extends AssignmentEndpoint {
+
+  @PostMapping("/InsecureDeserialization/task")
+  @ResponseBody
+  public AttackResult completed(@RequestParam String token) throws IOException {
+    String b64token;
+    long before;
+    long after;
+    int delay;
+
+    b64token = token.replace('-', '+').replace('_', '/');
+
+    try (ObjectInputStream ois =
+        createSafeObjectInputStream(new ByteArrayInputStream(Base64.getDecoder().decode(b64token)))) {
+      before = System.currentTimeMillis();
+      Object o = ois.readObject();
+      if (!(o instanceof VulnerableTaskHolder)) {
+        if (o instanceof String) {
+          return failed(this).feedback("insecure-deserialization.stringobject").build();
+        }
+        return failed(this).feedback("insecure-deserialization.wrongobject").build();
+      }
+      after = System.currentTimeMillis();
+    } catch (InvalidClassException e) {
+      return failed(this).feedback("insecure-deserialization.invalidversion").build();
+    } catch (IllegalArgumentException e) {
+      return failed(this).feedback("insecure-deserialization.expired").build();
+    } catch (Exception e) {
+      return failed(this).feedback("insecure-deserialization.invalidversion").build();
+    }
+
+    delay = (int) (after - before);
+    if (delay > 7000) {
+      return failed(this).build();
+    }
+    if (delay < 3000) {
+      return failed(this).build();
+    }
+    return success(this).build();
+  }
+}

core-codemods/src/test/resources/sonar-object-deserialization-s5135/InsecureDeserializationTask.java.before

@@ -0,0 +1,85 @@
+/*
+ * This file is part of WebGoat, an Open Web Application Security Project utility. For details, please see http://www.owasp.org/
+ *
+ * Copyright (c) 2002 - 2019 Bruce Mayhew
+ *
+ * This program is free software; you can redistribute it and/or modify it under the terms of the
+ * GNU General Public License as published by the Free Software Foundation; either version 2 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without
+ * even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along with this program; if
+ * not, write to the Free Software Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA
+ * 02111-1307, USA.
+ *
+ * Getting Source ==============
+ *
+ * Source for this application is maintained at https://github.com/WebGoat/WebGoat, a repository for free software projects.
+ */
+
+package org.owasp.webgoat.lessons.deserialization;
+
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InvalidClassException;
+import java.io.ObjectInputStream;
+import java.util.Base64;
+import org.dummy.insecure.framework.VulnerableTaskHolder;
+import org.owasp.webgoat.container.assignments.AssignmentEndpoint;
+import org.owasp.webgoat.container.assignments.AssignmentHints;
+import org.owasp.webgoat.container.assignments.AttackResult;
+import org.springframework.web.bind.annotation.PostMapping;
+import org.springframework.web.bind.annotation.RequestParam;
+import org.springframework.web.bind.annotation.ResponseBody;
+import org.springframework.web.bind.annotation.RestController;
+
+@RestController
+@AssignmentHints({
+  "insecure-deserialization.hints.1",
+  "insecure-deserialization.hints.2",
+  "insecure-deserialization.hints.3"
+})
+public class InsecureDeserializationTask extends AssignmentEndpoint {
+
+  @PostMapping("/InsecureDeserialization/task")
+  @ResponseBody
+  public AttackResult completed(@RequestParam String token) throws IOException {
+    String b64token;
+    long before;
+    long after;
+    int delay;
+
+    b64token = token.replace('-', '+').replace('_', '/');
+
+    try (ObjectInputStream ois =
+        new ObjectInputStream(new ByteArrayInputStream(Base64.getDecoder().decode(b64token)))) {
+      before = System.currentTimeMillis();
+      Object o = ois.readObject();
+      if (!(o instanceof VulnerableTaskHolder)) {
+        if (o instanceof String) {
+          return failed(this).feedback("insecure-deserialization.stringobject").build();
+        }
+        return failed(this).feedback("insecure-deserialization.wrongobject").build();
+      }
+      after = System.currentTimeMillis();
+    } catch (InvalidClassException e) {
+      return failed(this).feedback("insecure-deserialization.invalidversion").build();
+    } catch (IllegalArgumentException e) {
+      return failed(this).feedback("insecure-deserialization.expired").build();
+    } catch (Exception e) {
+      return failed(this).feedback("insecure-deserialization.invalidversion").build();
+    }
+
+    delay = (int) (after - before);
+    if (delay > 7000) {
+      return failed(this).build();
+    }
+    if (delay < 3000) {
+      return failed(this).build();
+    }
+    return success(this).build();
+  }
+}