From e3bada67dd6bcabcdeb4c61304360aa78054abf7 Mon Sep 17 00:00:00 2001 From: Philippe Date: Thu, 24 Jan 2019 23:35:44 -0200 Subject: [PATCH] BAEL-1381 --- sql-injection-samples/.gitignore | 25 +++ sql-injection-samples/pom.xml | 61 +++++++ .../examples/security/sql/AccountDAO.java | 169 ++++++++++++++++++ .../examples/security/sql/AccountDTO.java | 16 ++ .../sql/SqlInjectionSamplesApplication.java | 14 ++ .../src/main/resources/application.properties | 0 .../resources/db/changelog/create-tables.xml | 19 ++ .../main/resources/db/master-changelog.xml | 8 + ...qlInjectionSamplesApplicationUnitTest.java | 60 +++++++ .../src/test/resources/application-test.yml | 6 + .../src/test/resources/data.sql | 4 + .../src/test/resources/schema.sql | 6 + 12 files changed, 388 insertions(+) create mode 100644 sql-injection-samples/.gitignore create mode 100644 sql-injection-samples/pom.xml create mode 100644 sql-injection-samples/src/main/java/com/baeldung/examples/security/sql/AccountDAO.java create mode 100644 sql-injection-samples/src/main/java/com/baeldung/examples/security/sql/AccountDTO.java create mode 100644 sql-injection-samples/src/main/java/com/baeldung/examples/security/sql/SqlInjectionSamplesApplication.java create mode 100644 sql-injection-samples/src/main/resources/application.properties create mode 100644 sql-injection-samples/src/main/resources/db/changelog/create-tables.xml create mode 100644 sql-injection-samples/src/main/resources/db/master-changelog.xml create mode 100644 sql-injection-samples/src/test/java/com/baeldung/examples/security/sql/SqlInjectionSamplesApplicationUnitTest.java create mode 100644 sql-injection-samples/src/test/resources/application-test.yml create mode 100644 sql-injection-samples/src/test/resources/data.sql create mode 100644 sql-injection-samples/src/test/resources/schema.sql diff --git a/sql-injection-samples/.gitignore b/sql-injection-samples/.gitignore new file mode 100644 index 0000000000..82eca336e3 --- /dev/null +++ b/sql-injection-samples/.gitignore @@ -0,0 +1,25 @@ +/target/ +!.mvn/wrapper/maven-wrapper.jar + +### STS ### +.apt_generated +.classpath +.factorypath +.project +.settings +.springBeans +.sts4-cache + +### IntelliJ IDEA ### +.idea +*.iws +*.iml +*.ipr + +### NetBeans ### +/nbproject/private/ +/build/ +/nbbuild/ +/dist/ +/nbdist/ +/.nb-gradle/ \ No newline at end of file diff --git a/sql-injection-samples/pom.xml b/sql-injection-samples/pom.xml new file mode 100644 index 0000000000..b6390ad387 --- /dev/null +++ b/sql-injection-samples/pom.xml @@ -0,0 +1,61 @@ + + + 4.0.0 + + + parent-boot-2 + com.baeldung + 0.0.1-SNAPSHOT + ../parent-boot-2 + + + com.baeldung + sql-injection-samples + 0.0.1-SNAPSHOT + sql-injection-samples + Sample SQL Injection tests + + + 1.8 + + + + + org.springframework.boot + spring-boot-starter-jdbc + + + + org.apache.derby + derby + runtime + + + org.springframework.boot + spring-boot-configuration-processor + true + + + org.springframework.boot + spring-boot-starter-test + test + + + org.projectlombok + lombok + provided + + + + + + + + org.springframework.boot + spring-boot-maven-plugin + + + + + diff --git a/sql-injection-samples/src/main/java/com/baeldung/examples/security/sql/AccountDAO.java b/sql-injection-samples/src/main/java/com/baeldung/examples/security/sql/AccountDAO.java new file mode 100644 index 0000000000..447dcc456d --- /dev/null +++ b/sql-injection-samples/src/main/java/com/baeldung/examples/security/sql/AccountDAO.java @@ -0,0 +1,169 @@ +/** + * + */ +package com.baeldung.examples.security.sql; + +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import javax.sql.DataSource; + +import org.springframework.stereotype.Component; + +/** + * @author Philippe + * + */ +@Component +public class AccountDAO { + + private final DataSource dataSource; + + public AccountDAO(DataSource dataSource) { + this.dataSource = dataSource; + } + + /** + * Return all accounts owned by a given customer,given his/her external id + * + * @param customerId + * @return + */ + public List unsafeFindAccountsByCustomerId(String customerId) { + + String sql = "select " + "customer_id,acc_number,branch_id,balance from Accounts where customer_id = '" + customerId + "'"; + + try (Connection c = dataSource.getConnection(); + ResultSet rs = c.createStatement() + .executeQuery(sql)) { + List accounts = new ArrayList<>(); + while (rs.next()) { + AccountDTO acc = AccountDTO.builder() + .customerId(rs.getString("customer_id")) + .branchId(rs.getString("branch_id")) + .accNumber(rs.getString("acc_number")) + .balance(rs.getBigDecimal("balance")) + .build(); + + accounts.add(acc); + } + + return accounts; + } catch (SQLException ex) { + throw new RuntimeException(ex); + } + } + + /** + * Return all accounts owned by a given customer,given his/her external id + * + * @param customerId + * @return + */ + public List safeFindAccountsByCustomerId(String customerId) { + + String sql = "select " + "customer_id,acc_number,branch_id,balance from Accounts where customer_id = ?"; + + try (Connection c = dataSource.getConnection(); PreparedStatement p = c.prepareStatement(sql)) { + p.setString(1, customerId); + ResultSet rs = p.executeQuery(); + List accounts = new ArrayList<>(); + while (rs.next()) { + AccountDTO acc = AccountDTO.builder() + .customerId(rs.getString("customerId")) + .branchId(rs.getString("branch_id")) + .accNumber(rs.getString("acc_number")) + .balance(rs.getBigDecimal("balance")) + .build(); + + accounts.add(acc); + } + return accounts; + } catch (SQLException ex) { + throw new RuntimeException(ex); + } + } + + private static final Set VALID_COLUMNS_FOR_ORDER_BY = Stream.of("acc_number", "branch_id", "balance") + .collect(Collectors.toCollection(HashSet::new)); + + /** + * Return all accounts owned by a given customer,given his/her external id + * + * @param customerId + * @return + */ + public List safeFindAccountsByCustomerId(String customerId, String orderBy) { + + String sql = "select " + "customer_id,acc_number,branch_id,balance from Accounts where customer_id = ? "; + + if (VALID_COLUMNS_FOR_ORDER_BY.contains(orderBy)) { + sql = sql + " order by " + orderBy; + } + else { + throw new IllegalArgumentException("Nice try!"); + } + + try (Connection c = dataSource.getConnection(); PreparedStatement p = c.prepareStatement(sql)) { + + p.setString(1, customerId); + ResultSet rs = p.executeQuery(); + List accounts = new ArrayList<>(); + while (rs.next()) { + AccountDTO acc = AccountDTO.builder() + .customerId(rs.getString("customerId")) + .branchId(rs.getString("branch_id")) + .accNumber(rs.getString("acc_number")) + .balance(rs.getBigDecimal("balance")) + .build(); + + accounts.add(acc); + } + + return accounts; + } catch (SQLException ex) { + throw new RuntimeException(ex); + } + } + + /** + * Invalid placeholder usage example + * + * @param tableName + * @return + */ + public List wrongCountRecordsByTableName(String tableName) { + + try (Connection c = dataSource.getConnection(); + PreparedStatement p = c.prepareStatement("select count(*) from ?")) { + + p.setString(1, tableName); + ResultSet rs = p.executeQuery(); + List accounts = new ArrayList<>(); + while (rs.next()) { + AccountDTO acc = AccountDTO.builder() + .customerId(rs.getString("customerId")) + .branchId(rs.getString("branch_id")) + .accNumber(rs.getString("acc_number")) + .balance(rs.getBigDecimal("balance")) + .build(); + + accounts.add(acc); + } + + return accounts; + } catch (SQLException ex) { + throw new RuntimeException(ex); + } + } + +} diff --git a/sql-injection-samples/src/main/java/com/baeldung/examples/security/sql/AccountDTO.java b/sql-injection-samples/src/main/java/com/baeldung/examples/security/sql/AccountDTO.java new file mode 100644 index 0000000000..2e6fa04af4 --- /dev/null +++ b/sql-injection-samples/src/main/java/com/baeldung/examples/security/sql/AccountDTO.java @@ -0,0 +1,16 @@ +package com.baeldung.examples.security.sql; + +import java.math.BigDecimal; + +import lombok.Builder; +import lombok.Data; + +@Data +@Builder +public class AccountDTO { + + private String customerId; + private String accNumber; + private String branchId; + private BigDecimal balance; +} diff --git a/sql-injection-samples/src/main/java/com/baeldung/examples/security/sql/SqlInjectionSamplesApplication.java b/sql-injection-samples/src/main/java/com/baeldung/examples/security/sql/SqlInjectionSamplesApplication.java new file mode 100644 index 0000000000..c1083ae3de --- /dev/null +++ b/sql-injection-samples/src/main/java/com/baeldung/examples/security/sql/SqlInjectionSamplesApplication.java @@ -0,0 +1,14 @@ +package com.baeldung.examples.security.sql; + +import org.springframework.boot.SpringApplication; +import org.springframework.boot.autoconfigure.SpringBootApplication; + +@SpringBootApplication +public class SqlInjectionSamplesApplication { + + public static void main(String[] args) { + SpringApplication.run(SqlInjectionSamplesApplication.class, args); + } + +} + diff --git a/sql-injection-samples/src/main/resources/application.properties b/sql-injection-samples/src/main/resources/application.properties new file mode 100644 index 0000000000..e69de29bb2 diff --git a/sql-injection-samples/src/main/resources/db/changelog/create-tables.xml b/sql-injection-samples/src/main/resources/db/changelog/create-tables.xml new file mode 100644 index 0000000000..a405c02049 --- /dev/null +++ b/sql-injection-samples/src/main/resources/db/changelog/create-tables.xml @@ -0,0 +1,19 @@ + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/sql-injection-samples/src/main/resources/db/master-changelog.xml b/sql-injection-samples/src/main/resources/db/master-changelog.xml new file mode 100644 index 0000000000..047ca2b314 --- /dev/null +++ b/sql-injection-samples/src/main/resources/db/master-changelog.xml @@ -0,0 +1,8 @@ + + + + \ No newline at end of file diff --git a/sql-injection-samples/src/test/java/com/baeldung/examples/security/sql/SqlInjectionSamplesApplicationUnitTest.java b/sql-injection-samples/src/test/java/com/baeldung/examples/security/sql/SqlInjectionSamplesApplicationUnitTest.java new file mode 100644 index 0000000000..1f37ba04b6 --- /dev/null +++ b/sql-injection-samples/src/test/java/com/baeldung/examples/security/sql/SqlInjectionSamplesApplicationUnitTest.java @@ -0,0 +1,60 @@ +package com.baeldung.examples.security.sql; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.List; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.test.context.ActiveProfiles; +import org.springframework.test.context.junit4.SpringRunner; + +import com.baeldung.examples.security.sql.AccountDAO; +import com.baeldung.examples.security.sql.AccountDTO; + +@RunWith(SpringRunner.class) +@SpringBootTest +@ActiveProfiles({ "test" }) +public class SqlInjectionSamplesApplicationUnitTest { + + @Autowired + private AccountDAO target; + + @Test + public void givenAVulnerableMethod_whenValidCustomerId_thenReturnSingleAccount() { + + List accounts = target.unsafeFindAccountsByCustomerId("C1"); + assertThat(accounts).isNotNull(); + assertThat(accounts).isNotEmpty(); + assertThat(accounts).hasSize(1); + } + + @Test + public void givenAVulnerableMethod_whenHackedCustomerId_thenReturnAllAccounts() { + + List accounts = target.unsafeFindAccountsByCustomerId("C1' or '1'='1"); + assertThat(accounts).isNotNull(); + assertThat(accounts).isNotEmpty(); + assertThat(accounts).hasSize(3); + } + + @Test + public void givenASafeMethod_whenHackedCustomerId_thenReturnNoAccounts() { + + List accounts = target.safeFindAccountsByCustomerId("C1' or '1'='1"); + assertThat(accounts).isNotNull(); + assertThat(accounts).isEmpty(); + } + + @Test(expected = IllegalArgumentException.class) + public void givenASafeMethod_whenInvalidOrderBy_thenThroweException() { + target.safeFindAccountsByCustomerId("C1", "INVALID"); + } + + @Test(expected = RuntimeException.class) + public void givenWrongPlaceholderUsageMethod_whenNormalCall_thenThrowsException() { + target.wrongCountRecordsByTableName("Accounts"); + } +} diff --git a/sql-injection-samples/src/test/resources/application-test.yml b/sql-injection-samples/src/test/resources/application-test.yml new file mode 100644 index 0000000000..d07ee10aee --- /dev/null +++ b/sql-injection-samples/src/test/resources/application-test.yml @@ -0,0 +1,6 @@ +# +# Test profile configuration +# +spring: + datasource: + initialization-mode: always diff --git a/sql-injection-samples/src/test/resources/data.sql b/sql-injection-samples/src/test/resources/data.sql new file mode 100644 index 0000000000..586618a07f --- /dev/null +++ b/sql-injection-samples/src/test/resources/data.sql @@ -0,0 +1,4 @@ +insert into Accounts(customer_id,acc_number,branch_id,balance) values ('C1','0001',1,1000.00); +insert into Accounts(customer_id,acc_number,branch_id,balance) values ('C2','0002',1,500.00); +insert into Accounts(customer_id,acc_number,branch_id,balance) values ('C3','0003',1,501.00); + diff --git a/sql-injection-samples/src/test/resources/schema.sql b/sql-injection-samples/src/test/resources/schema.sql new file mode 100644 index 0000000000..bfb0ae8059 --- /dev/null +++ b/sql-injection-samples/src/test/resources/schema.sql @@ -0,0 +1,6 @@ +create table Accounts ( + customer_id varchar(16) not null, + acc_number varchar(16) not null, + branch_id decimal(8,0), + balance decimal(16,4) +);