From 01ec23b96d3b5b034d39cdac80ed47a489f1a980 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Tue, 28 Apr 2026 21:57:56 +0000 Subject: [PATCH 1/4] fix: remediate critical security vulnerabilities (CSRF, negative amounts, race conditions) - Enable CSRF protection in SecurityConfig (was globally disabled) - Add positive amount validation on deposit, withdraw, transfer operations - Add @Transactional and pessimistic locking to prevent race conditions - Add username/password input validation on registration - Update Thymeleaf templates to use th:action for CSRF token injection - Add error handling for deposit controller endpoint - Add H2 test profile for local security testing - Add min=1 constraint on HTML amount input fields Co-Authored-By: Angela Lin --- pom.xml | 5 ++ .../bankapp/config/SecurityConfig.java | 2 +- .../bankapp/controller/BankController.java | 12 +++- .../bankapp/repository/AccountRepository.java | 7 ++ .../bankapp/service/AccountService.java | 68 +++++++++++++------ src/main/resources/application-h2.properties | 12 ++++ src/main/resources/templates/dashboard.html | 12 ++-- src/main/resources/templates/login.html | 2 +- src/main/resources/templates/register.html | 2 +- 9 files changed, 89 insertions(+), 33 deletions(-) create mode 100644 src/main/resources/application-h2.properties diff --git a/pom.xml b/pom.xml index fc5bfeac..ba964386 100644 --- a/pom.xml +++ b/pom.xml @@ -57,6 +57,11 @@ 8.0.33 runtime + + com.h2database + h2 + runtime + org.springframework.boot spring-boot-starter-test diff --git a/src/main/java/com/example/bankapp/config/SecurityConfig.java b/src/main/java/com/example/bankapp/config/SecurityConfig.java index 4dbd1572..0b108284 100644 --- a/src/main/java/com/example/bankapp/config/SecurityConfig.java +++ b/src/main/java/com/example/bankapp/config/SecurityConfig.java @@ -27,7 +27,7 @@ public static PasswordEncoder passwordEncoder() { @Bean public SecurityFilterChain securityFilterChain(HttpSecurity http) throws Exception { http - .csrf(csrf -> csrf.disable()) + .csrf(csrf -> {}) .authorizeHttpRequests(authz -> authz .requestMatchers("/register").permitAll() .anyRequest().authenticated() diff --git a/src/main/java/com/example/bankapp/controller/BankController.java b/src/main/java/com/example/bankapp/controller/BankController.java index 19fcded7..62724436 100644 --- a/src/main/java/com/example/bankapp/controller/BankController.java +++ b/src/main/java/com/example/bankapp/controller/BankController.java @@ -48,10 +48,18 @@ public String login() { } @PostMapping("/deposit") - public String deposit(@RequestParam BigDecimal amount) { + public String deposit(@RequestParam BigDecimal amount, Model model) { String username = SecurityContextHolder.getContext().getAuthentication().getName(); Account account = accountService.findAccountByUsername(username); - accountService.deposit(account, amount); + + try { + accountService.deposit(account, amount); + } catch (RuntimeException e) { + model.addAttribute("error", e.getMessage()); + model.addAttribute("account", account); + return "dashboard"; + } + return "redirect:/dashboard"; } diff --git a/src/main/java/com/example/bankapp/repository/AccountRepository.java b/src/main/java/com/example/bankapp/repository/AccountRepository.java index 72553370..07a78ab0 100644 --- a/src/main/java/com/example/bankapp/repository/AccountRepository.java +++ b/src/main/java/com/example/bankapp/repository/AccountRepository.java @@ -1,10 +1,17 @@ package com.example.bankapp.repository; import com.example.bankapp.model.Account; +import jakarta.persistence.LockModeType; import org.springframework.data.jpa.repository.JpaRepository; +import org.springframework.data.jpa.repository.Lock; +import org.springframework.data.jpa.repository.Query; import java.util.Optional; public interface AccountRepository extends JpaRepository { Optional findByUsername(String username); + + @Lock(LockModeType.PESSIMISTIC_WRITE) + @Query("SELECT a FROM Account a WHERE a.username = :username") + Optional findByUsernameForUpdate(String username); } diff --git a/src/main/java/com/example/bankapp/service/AccountService.java b/src/main/java/com/example/bankapp/service/AccountService.java index 5d7d90ec..78565037 100644 --- a/src/main/java/com/example/bankapp/service/AccountService.java +++ b/src/main/java/com/example/bankapp/service/AccountService.java @@ -12,6 +12,7 @@ import org.springframework.security.core.userdetails.UsernameNotFoundException; import org.springframework.security.crypto.password.PasswordEncoder; import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; import java.math.BigDecimal; import java.time.LocalDateTime; @@ -35,44 +36,63 @@ public Account findAccountByUsername(String username) { return accountRepository.findByUsername(username).orElseThrow(() -> new RuntimeException("Account not found")); } + @Transactional public Account registerAccount(String username, String password) { + if (username == null || !username.matches("^[a-zA-Z0-9_]{3,50}$")) { + throw new RuntimeException("Username must be 3-50 alphanumeric characters or underscores"); + } + if (password == null || password.length() < 8) { + throw new RuntimeException("Password must be at least 8 characters"); + } if (accountRepository.findByUsername(username).isPresent()) { - throw new RuntimeException("Username already exists"); + throw new RuntimeException("Registration failed. Please try a different username."); } Account account = new Account(); account.setUsername(username); - account.setPassword(passwordEncoder.encode(password)); // Encrypt password - account.setBalance(BigDecimal.ZERO); // Initial balance set to 0 + account.setPassword(passwordEncoder.encode(password)); + account.setBalance(BigDecimal.ZERO); return accountRepository.save(account); } + @Transactional public void deposit(Account account, BigDecimal amount) { - account.setBalance(account.getBalance().add(amount)); - accountRepository.save(account); + if (amount == null || amount.compareTo(BigDecimal.ZERO) <= 0) { + throw new RuntimeException("Amount must be positive"); + } + Account locked = accountRepository.findByUsernameForUpdate(account.getUsername()) + .orElseThrow(() -> new RuntimeException("Account not found")); + locked.setBalance(locked.getBalance().add(amount)); + accountRepository.save(locked); Transaction transaction = new Transaction( amount, "Deposit", LocalDateTime.now(), - account + locked ); transactionRepository.save(transaction); } + @Transactional public void withdraw(Account account, BigDecimal amount) { - if (account.getBalance().compareTo(amount) < 0) { + if (amount == null || amount.compareTo(BigDecimal.ZERO) <= 0) { + throw new RuntimeException("Amount must be positive"); + } + Account locked = accountRepository.findByUsernameForUpdate(account.getUsername()) + .orElseThrow(() -> new RuntimeException("Account not found")); + if (locked.getBalance().compareTo(amount) < 0) { throw new RuntimeException("Insufficient funds"); } - account.setBalance(account.getBalance().subtract(amount)); - accountRepository.save(account); + locked.setBalance(locked.getBalance().subtract(amount)); + accountRepository.save(locked); Transaction transaction = new Transaction( amount, "Withdrawal", LocalDateTime.now(), - account + locked ); transactionRepository.save(transaction); } @@ -100,36 +120,40 @@ public Collection authorities() { return Arrays.asList(new SimpleGrantedAuthority("USER")); } + @Transactional public void transferAmount(Account fromAccount, String toUsername, BigDecimal amount) { - if (fromAccount.getBalance().compareTo(amount) < 0) { + if (amount == null || amount.compareTo(BigDecimal.ZERO) <= 0) { + throw new RuntimeException("Amount must be positive"); + } + Account lockedFrom = accountRepository.findByUsernameForUpdate(fromAccount.getUsername()) + .orElseThrow(() -> new RuntimeException("Account not found")); + if (lockedFrom.getBalance().compareTo(amount) < 0) { throw new RuntimeException("Insufficient funds"); } - Account toAccount = accountRepository.findByUsername(toUsername) + Account lockedTo = accountRepository.findByUsernameForUpdate(toUsername) .orElseThrow(() -> new RuntimeException("Recipient account not found")); - // Deduct from sender's account - fromAccount.setBalance(fromAccount.getBalance().subtract(amount)); - accountRepository.save(fromAccount); + lockedFrom.setBalance(lockedFrom.getBalance().subtract(amount)); + accountRepository.save(lockedFrom); - // Add to recipient's account - toAccount.setBalance(toAccount.getBalance().add(amount)); - accountRepository.save(toAccount); + lockedTo.setBalance(lockedTo.getBalance().add(amount)); + accountRepository.save(lockedTo); // Create transaction records for both accounts Transaction debitTransaction = new Transaction( amount, - "Transfer Out to " + toAccount.getUsername(), + "Transfer Out to " + lockedTo.getUsername(), LocalDateTime.now(), - fromAccount + lockedFrom ); transactionRepository.save(debitTransaction); Transaction creditTransaction = new Transaction( amount, - "Transfer In from " + fromAccount.getUsername(), + "Transfer In from " + lockedFrom.getUsername(), LocalDateTime.now(), - toAccount + lockedTo ); transactionRepository.save(creditTransaction); } diff --git a/src/main/resources/application-h2.properties b/src/main/resources/application-h2.properties new file mode 100644 index 00000000..0b8dc537 --- /dev/null +++ b/src/main/resources/application-h2.properties @@ -0,0 +1,12 @@ +spring.application.name=bankapp +spring.datasource.url=jdbc:h2:mem:testdb;DB_CLOSE_DELAY=-1 +spring.datasource.driverClassName=org.h2.Driver +spring.datasource.username=sa +spring.datasource.password= +spring.datasource.driver-class-name=org.h2.Driver +spring.jpa.database-platform=org.hibernate.dialect.H2Dialect +spring.jpa.properties.hibernate.dialect=org.hibernate.dialect.H2Dialect +spring.jpa.hibernate.ddl-auto=create-drop +spring.jpa.show-sql=false +spring.h2.console.enabled=true +spring.jpa.properties.hibernate.globally_quoted_identifiers=true diff --git a/src/main/resources/templates/dashboard.html b/src/main/resources/templates/dashboard.html index 23a0cc7f..9a3203ff 100644 --- a/src/main/resources/templates/dashboard.html +++ b/src/main/resources/templates/dashboard.html @@ -141,10 +141,10 @@

Account Details

Deposit
-
+
- +
@@ -157,10 +157,10 @@

Account Details

Withdraw
-
+
- +
@@ -173,14 +173,14 @@

Account Details

Transfer Money
-
+
- +
diff --git a/src/main/resources/templates/login.html b/src/main/resources/templates/login.html index 7b40190d..412a68eb 100644 --- a/src/main/resources/templates/login.html +++ b/src/main/resources/templates/login.html @@ -105,7 +105,7 @@

Login

-
+
diff --git a/src/main/resources/templates/register.html b/src/main/resources/templates/register.html index e410b74e..7555094b 100644 --- a/src/main/resources/templates/register.html +++ b/src/main/resources/templates/register.html @@ -105,7 +105,7 @@

Register a New Account

- +
From 25b3be0cd3d2a518d3d84b2c9ced829a2de1cb97 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Tue, 28 Apr 2026 21:59:10 +0000 Subject: [PATCH 2/4] fix: change H2 dependency scope to test to resolve Snyk license check Co-Authored-By: Angela Lin --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index ba964386..eed97c9e 100644 --- a/pom.xml +++ b/pom.xml @@ -60,7 +60,7 @@ com.h2database h2 - runtime + test org.springframework.boot From 3bbd5bd68ddf631f638f966a69cc81110f1644c1 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Tue, 28 Apr 2026 22:03:04 +0000 Subject: [PATCH 3/4] fix: address Devin Review findings - deterministic lock ordering and dynamic error messages - Fix ABBA deadlock risk in transferAmount by acquiring locks in alphabetical username order - Fix register.html to display dynamic error messages instead of hardcoded 'User already present' Co-Authored-By: Angela Lin --- .../bankapp/service/AccountService.java | 25 +++++++++++++++---- src/main/resources/templates/register.html | 4 +-- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/src/main/java/com/example/bankapp/service/AccountService.java b/src/main/java/com/example/bankapp/service/AccountService.java index 78565037..dd9342f7 100644 --- a/src/main/java/com/example/bankapp/service/AccountService.java +++ b/src/main/java/com/example/bankapp/service/AccountService.java @@ -125,15 +125,30 @@ public void transferAmount(Account fromAccount, String toUsername, BigDecimal am if (amount == null || amount.compareTo(BigDecimal.ZERO) <= 0) { throw new RuntimeException("Amount must be positive"); } - Account lockedFrom = accountRepository.findByUsernameForUpdate(fromAccount.getUsername()) - .orElseThrow(() -> new RuntimeException("Account not found")); + String senderName = fromAccount.getUsername(); + String recipientName = toUsername; + + // Acquire locks in deterministic order to prevent ABBA deadlock + Account first, second; + if (senderName.compareTo(recipientName) < 0) { + first = accountRepository.findByUsernameForUpdate(senderName) + .orElseThrow(() -> new RuntimeException("Account not found")); + second = accountRepository.findByUsernameForUpdate(recipientName) + .orElseThrow(() -> new RuntimeException("Recipient account not found")); + } else { + second = accountRepository.findByUsernameForUpdate(recipientName) + .orElseThrow(() -> new RuntimeException("Recipient account not found")); + first = accountRepository.findByUsernameForUpdate(senderName) + .orElseThrow(() -> new RuntimeException("Account not found")); + } + + Account lockedFrom = senderName.equals(first.getUsername()) ? first : second; + Account lockedTo = senderName.equals(first.getUsername()) ? second : first; + if (lockedFrom.getBalance().compareTo(amount) < 0) { throw new RuntimeException("Insufficient funds"); } - Account lockedTo = accountRepository.findByUsernameForUpdate(toUsername) - .orElseThrow(() -> new RuntimeException("Recipient account not found")); - lockedFrom.setBalance(lockedFrom.getBalance().subtract(amount)); accountRepository.save(lockedFrom); diff --git a/src/main/resources/templates/register.html b/src/main/resources/templates/register.html index 7555094b..9ddd4cec 100644 --- a/src/main/resources/templates/register.html +++ b/src/main/resources/templates/register.html @@ -118,9 +118,7 @@

Register a New Account

Already have an account? Login here

-
- User already present. -
+