Skip to content

Commit cc9483a

Browse files
nahsranahsra
committed
create basics
1 parent 9aca74c commit cc9483a

File tree

18 files changed

+3295
-24
lines changed

18 files changed

+3295
-24
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ public static List<Class<? extends CodeChanger>> asList() {
1717
AddMissingOverrideCodemod.class,
1818
AvoidImplicitPublicConstructorCodemod.class,
1919
DeclareVariableOnSeparateLineCodemod.class,
20+
DefectDojoSqlInjectionCodemod.class,
2021
DefineConstantForLiteralCodemod.class,
2122
DisableAutomaticDirContextDeserializationCodemod.class,
2223
FixRedundantStaticOnEnumCodemod.class,

‎core-codemods/src/main/java/io/codemodder/codemods/DefectDojoSqlInjectionCodemod.java‎

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@
33
import com.github.javaparser.ast.CompilationUnit;
44
import com.github.javaparser.ast.expr.MethodCallExpr;
55
import io.codemodder.*;
6+
import io.codemodder.codetf.DetectionTool;
67
import io.codemodder.codetf.DetectorFinding;
8+
import io.codemodder.codetf.DetectorRule;
79
import io.codemodder.javaparser.JavaParserChanger;
810
import io.codemodder.providers.defectdojo.DefectDojoScan;
911
import io.codemodder.providers.defectdojo.Finding;
@@ -19,7 +21,8 @@
1921
reviewGuidance = ReviewGuidance.MERGE_AFTER_REVIEW,
2022
executionPriority = CodemodExecutionPriority.HIGH,
2123
importance = Importance.HIGH)
22-
public final class DefectDojoSqlInjectionCodemod extends JavaParserChanger {
24+
public final class DefectDojoSqlInjectionCodemod extends JavaParserChanger
25+
implements FixOnlyCodeChanger {
2326

2427
private final RuleFindings findings;
2528

@@ -30,6 +33,16 @@ public DefectDojoSqlInjectionCodemod(
3033
this.findings = Objects.requireNonNull(findings);
3134
}
3235

36+
@Override
37+
public DetectionTool getDetectionTool() {
38+
DetectorRule semgrepSqliRule =
39+
new DetectorRule(
40+
"java.lang.security.audit.sqli.jdbc-sqli.jdbc-sqli",
41+
"java.lang.security.audit.sqli.jdbc-sqli.jdbc-sqli",
42+
null);
43+
return new DetectionTool("DefectDojo", semgrepSqliRule, List.of());
44+
}
45+
3346
@Override
3447
public CodemodFileScanningResult visit(
3548
final CodemodInvocationContext context, final CompilationUnit cu) {
@@ -41,8 +54,7 @@ public CodemodFileScanningResult visit(
4154
return CodemodFileScanningResult.none();
4255
}
4356

44-
List<DetectorFinding> fixed = new ArrayList<>();
45-
List<DetectorFinding> allUnfixable = new ArrayList<>();
57+
List<DetectorFinding> allFindings = new ArrayList<>();
4658

4759
List<CodemodChange> changes = new ArrayList<>();
4860
for (Finding finding : findingsForThisPath) {
@@ -51,7 +63,7 @@ public CodemodFileScanningResult visit(
5163
if (line == null) {
5264
DetectorFinding unfixableFinding =
5365
new DetectorFinding(id, false, "No line number provided");
54-
allUnfixable.add(unfixableFinding);
66+
allFindings.add(unfixableFinding);
5567
continue;
5668
}
5769

@@ -64,15 +76,15 @@ public CodemodFileScanningResult visit(
6476
if (supportedSqlMethodCallsOnThatLine.isEmpty()) {
6577
DetectorFinding unfixableFinding =
6678
new DetectorFinding(id, false, "No supported SQL methods found on the given line");
67-
allUnfixable.add(unfixableFinding);
79+
allFindings.add(unfixableFinding);
6880
continue;
6981
}
7082

7183
if (supportedSqlMethodCallsOnThatLine.size() > 1) {
7284
DetectorFinding unfixableFinding =
7385
new DetectorFinding(
7486
id, false, "Multiple supported SQL methods found on the given line");
75-
allUnfixable.add(unfixableFinding);
87+
allFindings.add(unfixableFinding);
7688
continue;
7789
}
7890

@@ -81,15 +93,15 @@ public CodemodFileScanningResult visit(
8193

8294
if (parameterizer.checkAndFix()) {
8395
DetectorFinding fixedFinding = new DetectorFinding(id, true, null);
84-
fixed.add(fixedFinding);
96+
allFindings.add(fixedFinding);
8597
changes.add(CodemodChange.from(line, "Fixes issue " + id + " by parameterizing SQL"));
8698
} else {
8799
DetectorFinding unfixableFinding =
88100
new DetectorFinding(id, false, "Fixing may have side effects");
89-
allUnfixable.add(unfixableFinding);
101+
allFindings.add(unfixableFinding);
90102
}
91103
}
92104

93-
return CodemodFileScanningResult.from(changes, allUnfixable);
105+
return CodemodFileScanningResult.from(changes, allFindings);
94106
}
95107
}

‎core-codemods/src/main/resources/io/codemodder/codemods/DefectDojoSqlInjectionCodemod/description.md‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
This change fixes SQL injections found in DefectDojo by parameterizing the given statement.
2+
3+
Without parameterization, developers must remember to escape inputs using the rules for that database. It's usually buggy, at the least -- and sometimes vulnerable.
4+
5+
Our changes look something like this:
6+
7+
```diff
8+
- Statement stmt = connection.createStatement();
9+
- ResultSet rs = stmt.executeQuery("SELECT * FROM users WHERE name = '" + user + "'");
10+
+ PreparedStatement stmt = connection.prepareStatement("SELECT * FROM users WHERE name = ?");
11+
+ stmt.setString(1, user);
12+
+ ResultSet rs = stmt.executeQuery();
13+
```

‎core-codemods/src/main/resources/io/codemodder/codemods/DefectDojoSqlInjectionCodemod/report.json‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
{
2+
"summary" : "Fixed SQL Injection",
3+
"change": "Parameterized SQL usage to prevent any bugs or vulnerabilities",
4+
"reviewGuidanceIJustification" : "Although there should be no functional differences, the rewrite here is complex and should be verified by a human.",
5+
"references" : [
6+
"https://cheatsheetseries.owasp.org/cheatsheets/SQL_Injection_Prevention_Cheat_Sheet.html",
7+
"https://cwe.mitre.org/data/definitions/89.html"
8+
]
9+
}

‎core-codemods/src/test/java/io/codemodder/codemods/DefectDojoSqlInjectionCodemodTest.java‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
package io.codemodder.codemods;
2+
3+
import io.codemodder.testutils.CodemodTestMixin;
4+
import io.codemodder.testutils.Metadata;
5+
import org.junit.jupiter.api.Nested;
6+
7+
final class DefectDojoSqlInjectionCodemodTest {
8+
9+
@Nested
10+
@Metadata(
11+
codemodType = DefectDojoSqlInjectionCodemod.class,
12+
testResourceDir = "defectdojo-sql-injection/SqlInjectionChallenge",
13+
renameTestFile =
14+
"src/main/java/org/owasp/webgoat/lessons/sqlinjection/advanced/SqlInjectionChallenge.java",
15+
dependencies = {})
16+
final class DefectDojoSqlInjectionChallengeCodemodTestMixin implements CodemodTestMixin {}
17+
18+
@Nested
19+
@Metadata(
20+
codemodType = DefectDojoSqlInjectionCodemod.class,
21+
testResourceDir = "defectdojo-sql-injection/SqlInjectionLesson8",
22+
renameTestFile =
23+
"src/main/java/org/owasp/webgoat/lessons/sqlinjection/introduction/SqlInjectionLesson8.java",
24+
dependencies = {})
25+
final class DefectDojoSqlInjectionLesson8CodemodTestMixin implements CodemodTestMixin {}
26+
}

‎core-codemods/src/test/resources/defectdojo-sql-injection/SqlInjectionChallenge/SqlInjectionChallenge.java.after‎

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
/*
2+
* This file is part of WebGoat, an Open Web Application Security Project utility. For details, please see http://www.owasp.org/
3+
*
4+
* Copyright (c) 2002 - 2019 Bruce Mayhew
5+
*
6+
* This program is free software; you can redistribute it and/or modify it under the terms of the
7+
* GNU General Public License as published by the Free Software Foundation; either version 2 of the
8+
* License, or (at your option) any later version.
9+
*
10+
* This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without
11+
* even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
12+
* General Public License for more details.
13+
*
14+
* You should have received a copy of the GNU General Public License along with this program; if
15+
* not, write to the Free Software Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA
16+
* 02111-1307, USA.
17+
*
18+
* Getting Source ==============
19+
*
20+
* Source for this application is maintained at https://github.com/WebGoat/WebGoat, a repository for free software projects.
21+
*/
22+
23+
package org.owasp.webgoat.lessons.sqlinjection.advanced;
24+
25+
import java.sql.*;
26+
import java.sql.PreparedStatement;
27+
import lombok.extern.slf4j.Slf4j;
28+
import org.owasp.webgoat.container.LessonDataSource;
29+
import org.owasp.webgoat.container.assignments.AssignmentEndpoint;
30+
import org.owasp.webgoat.container.assignments.AssignmentHints;
31+
import org.owasp.webgoat.container.assignments.AttackResult;
32+
import org.springframework.util.StringUtils;
33+
import org.springframework.web.bind.annotation.PutMapping;
34+
import org.springframework.web.bind.annotation.RequestParam;
35+
import org.springframework.web.bind.annotation.ResponseBody;
36+
import org.springframework.web.bind.annotation.RestController;
37+
38+
/**
39+
* @author nbaars
40+
* @since 4/8/17.
41+
*/
42+
@RestController
43+
@AssignmentHints(
44+
value = {"SqlInjectionChallenge1", "SqlInjectionChallenge2", "SqlInjectionChallenge3"})
45+
@Slf4j
46+
public class SqlInjectionChallenge extends AssignmentEndpoint {
47+
48+
private final LessonDataSource dataSource;
49+
50+
public SqlInjectionChallenge(LessonDataSource dataSource) {
51+
this.dataSource = dataSource;
52+
}
53+
54+
@PutMapping("/SqlInjectionAdvanced/challenge")
55+
// assignment path is bounded to class so we use different http method :-)
56+
@ResponseBody
57+
public AttackResult registerNewUser(
58+
@RequestParam String username_reg,
59+
@RequestParam String email_reg,
60+
@RequestParam String password_reg)
61+
throws Exception {
62+
AttackResult attackResult = checkArguments(username_reg, email_reg, password_reg);
63+
64+
if (attackResult == null) {
65+
66+
try (Connection connection = dataSource.getConnection()) {
67+
String checkUserQuery =
68+
"select userid from sql_challenge_users where userid = ?";
69+
PreparedStatement statement = connection.prepareStatement(checkUserQuery);
70+
statement.setString(1, username_reg);
71+
ResultSet resultSet = statement.execute();
72+
73+
if (resultSet.next()) {
74+
if (username_reg.contains("tom'")) {
75+
attackResult = success(this).feedback("user.exists").build();
76+
} else {
77+
attackResult = failed(this).feedback("user.exists").feedbackArgs(username_reg).build();
78+
}
79+
} else {
80+
PreparedStatement preparedStatement =
81+
connection.prepareStatement("INSERT INTO sql_challenge_users VALUES (?, ?, ?)");
82+
preparedStatement.setString(1, username_reg);
83+
preparedStatement.setString(2, email_reg);
84+
preparedStatement.setString(3, password_reg);
85+
preparedStatement.execute();
86+
attackResult = success(this).feedback("user.created").feedbackArgs(username_reg).build();
87+
}
88+
} catch (SQLException e) {
89+
attackResult = failed(this).output("Something went wrong").build();
90+
}
91+
}
92+
return attackResult;
93+
}
94+
95+
private AttackResult checkArguments(String username_reg, String email_reg, String password_reg) {
96+
if (StringUtils.isEmpty(username_reg)
97+
|| StringUtils.isEmpty(email_reg)
98+
|| StringUtils.isEmpty(password_reg)) {
99+
return failed(this).feedback("input.invalid").build();
100+
}
101+
if (username_reg.length() > 250 || email_reg.length() > 30 || password_reg.length() > 30) {
102+
return failed(this).feedback("input.invalid").build();
103+
}
104+
return null;
105+
}
106+
}

‎core-codemods/src/test/resources/defectdojo-sql-injection/SqlInjectionChallenge/SqlInjectionChallenge.java.before‎

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
/*
2+
* This file is part of WebGoat, an Open Web Application Security Project utility. For details, please see http://www.owasp.org/
3+
*
4+
* Copyright (c) 2002 - 2019 Bruce Mayhew
5+
*
6+
* This program is free software; you can redistribute it and/or modify it under the terms of the
7+
* GNU General Public License as published by the Free Software Foundation; either version 2 of the
8+
* License, or (at your option) any later version.
9+
*
10+
* This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without
11+
* even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
12+
* General Public License for more details.
13+
*
14+
* You should have received a copy of the GNU General Public License along with this program; if
15+
* not, write to the Free Software Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA
16+
* 02111-1307, USA.
17+
*
18+
* Getting Source ==============
19+
*
20+
* Source for this application is maintained at https://github.com/WebGoat/WebGoat, a repository for free software projects.
21+
*/
22+
23+
package org.owasp.webgoat.lessons.sqlinjection.advanced;
24+
25+
import java.sql.*;
26+
import lombok.extern.slf4j.Slf4j;
27+
import org.owasp.webgoat.container.LessonDataSource;
28+
import org.owasp.webgoat.container.assignments.AssignmentEndpoint;
29+
import org.owasp.webgoat.container.assignments.AssignmentHints;
30+
import org.owasp.webgoat.container.assignments.AttackResult;
31+
import org.springframework.util.StringUtils;
32+
import org.springframework.web.bind.annotation.PutMapping;
33+
import org.springframework.web.bind.annotation.RequestParam;
34+
import org.springframework.web.bind.annotation.ResponseBody;
35+
import org.springframework.web.bind.annotation.RestController;
36+
37+
/**
38+
* @author nbaars
39+
* @since 4/8/17.
40+
*/
41+
@RestController
42+
@AssignmentHints(
43+
value = {"SqlInjectionChallenge1", "SqlInjectionChallenge2", "SqlInjectionChallenge3"})
44+
@Slf4j
45+
public class SqlInjectionChallenge extends AssignmentEndpoint {
46+
47+
private final LessonDataSource dataSource;
48+
49+
public SqlInjectionChallenge(LessonDataSource dataSource) {
50+
this.dataSource = dataSource;
51+
}
52+
53+
@PutMapping("/SqlInjectionAdvanced/challenge")
54+
// assignment path is bounded to class so we use different http method :-)
55+
@ResponseBody
56+
public AttackResult registerNewUser(
57+
@RequestParam String username_reg,
58+
@RequestParam String email_reg,
59+
@RequestParam String password_reg)
60+
throws Exception {
61+
AttackResult attackResult = checkArguments(username_reg, email_reg, password_reg);
62+
63+
if (attackResult == null) {
64+
65+
try (Connection connection = dataSource.getConnection()) {
66+
String checkUserQuery =
67+
"select userid from sql_challenge_users where userid = '" + username_reg + "'";
68+
Statement statement = connection.createStatement();
69+
ResultSet resultSet = statement.executeQuery(checkUserQuery);
70+
71+
if (resultSet.next()) {
72+
if (username_reg.contains("tom'")) {
73+
attackResult = success(this).feedback("user.exists").build();
74+
} else {
75+
attackResult = failed(this).feedback("user.exists").feedbackArgs(username_reg).build();
76+
}
77+
} else {
78+
PreparedStatement preparedStatement =
79+
connection.prepareStatement("INSERT INTO sql_challenge_users VALUES (?, ?, ?)");
80+
preparedStatement.setString(1, username_reg);
81+
preparedStatement.setString(2, email_reg);
82+
preparedStatement.setString(3, password_reg);
83+
preparedStatement.execute();
84+
attackResult = success(this).feedback("user.created").feedbackArgs(username_reg).build();
85+
}
86+
} catch (SQLException e) {
87+
attackResult = failed(this).output("Something went wrong").build();
88+
}
89+
}
90+
return attackResult;
91+
}
92+
93+
private AttackResult checkArguments(String username_reg, String email_reg, String password_reg) {
94+
if (StringUtils.isEmpty(username_reg)
95+
|| StringUtils.isEmpty(email_reg)
96+
|| StringUtils.isEmpty(password_reg)) {
97+
return failed(this).feedback("input.invalid").build();
98+
}
99+
if (username_reg.length() > 250 || email_reg.length() > 30 || password_reg.length() > 30) {
100+
return failed(this).feedback("input.invalid").build();
101+
}
102+
return null;
103+
}
104+
}

0 commit comments

Comments
 (0)