fix: remediate critical security vulnerabilities (CSRF, negative amounts, race conditions)

pom.xml

@@ -57,6 +57,11 @@
 			<version>8.0.33</version>
 			<scope>runtime</scope>
 		</dependency>
+		<dependency>
+			<groupId>com.h2database</groupId>
+			<artifactId>h2</artifactId>
+			<scope>test</scope>
+		</dependency>
 		<dependency>
 			<groupId>org.springframework.boot</groupId>
 			<artifactId>spring-boot-starter-test</artifactId>

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()

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";
     }
 

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<Account, Long> {
     Optional<Account> findByUsername(String username);
+
+    @Lock(LockModeType.PESSIMISTIC_WRITE)
+    @Query("SELECT a FROM Account a WHERE a.username = :username")
+    Optional<Account> findByUsernameForUpdate(String username);
 }

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,59 @@ public Collection<? extends GrantedAuthority> authorities() {
         return Arrays.asList(new SimpleGrantedAuthority("USER"));
     }
 
+    @Transactional
     public void transferAmount(Account fromAccount, String toUsername, BigDecimal amount) {
-        if (fromAccount.getBalance().compareTo(amount) < 0) {
-            throw new RuntimeException("Insufficient funds");
+        if (amount == null || amount.compareTo(BigDecimal.ZERO) <= 0) {
+            throw new RuntimeException("Amount must be positive");
+        }
+        String senderName = fromAccount.getUsername();
+        String recipientName = toUsername;
+
+        if (senderName.equals(recipientName)) {
+            throw new RuntimeException("Cannot transfer to yourself");
+        }
+
+        // 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 toAccount = accountRepository.findByUsername(toUsername)
-                .orElseThrow(() -> new RuntimeException("Recipient 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");
+        }
 
-        // 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);
     }

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

src/main/resources/templates/dashboard.html

@@ -141,10 +141,10 @@ <h4>Account Details</h4>
                 Deposit
             </button>
             <div class="collapse mt-3" id="depositForm">
-                <form method="post" action="/deposit" class="form-container">
+                <form method="post" th:action="@{/deposit}" class="form-container">
                     <div class="form-group">
                         <label>Amount:</label>
-                        <input type="number" class="form-control" name="amount" required />
+                        <input type="number" class="form-control" name="amount" min="1" step="0.01" required />
                     </div>
                     <button type="submit" class="btn btn-block btn-success">Submit</button>
                 </form>
@@ -157,10 +157,10 @@ <h4>Account Details</h4>
                 Withdraw
             </button>
             <div class="collapse mt-3" id="withdrawForm">
-                <form method="post" action="/withdraw" class="form-container">
+                <form method="post" th:action="@{/withdraw}" class="form-container">
                     <div class="form-group">
                         <label>Amount:</label>
-                        <input type="number" class="form-control" name="amount" required />
+                        <input type="number" class="form-control" name="amount" min="1" step="0.01" required />
                     </div>
                     <button type="submit" class="btn btn-block btn-success">Submit</button>
                 </form>
@@ -173,14 +173,14 @@ <h4>Account Details</h4>
                 Transfer Money
             </button>
             <div class="collapse mt-3" id="transferForm">
-                <form method="post" action="/transfer" class="form-container">
+                <form method="post" th:action="@{/transfer}" class="form-container">
                     <div class="form-group">
                         <label>Recipient Username:</label>
                         <input type="text" class="form-control" name="toUsername" required />
                     </div>
                     <div class="form-group">
                         <label>Amount:</label>
-                        <input type="number" class="form-control" name="amount" required />
+                        <input type="number" class="form-control" name="amount" min="1" step="0.01" required />
                     </div>
                     <button type="submit" class="btn btn-block btn-success">Submit</button>
                 </form>

src/main/resources/templates/login.html

@@ -105,7 +105,7 @@
 
 <div class="container">
     <h2 class="text-left">Login</h2>
-    <form method="post" action="/login">
+    <form method="post" th:action="@{/login}">
         <div class="form-group">
             <label>Username:</label>
             <input type="text" class="form-control" name="username" required />

src/main/resources/templates/register.html

@@ -105,7 +105,7 @@
 
 <div class="container">
     <h2 class="text-left">Register a New Account</h2>
-    <form method="post" action="/register">
+    <form method="post" th:action="@{/register}">
         <div class="form-group">
             <label>Username:</label>
             <input type="text" class="form-control" name="username" required />
@@ -118,9 +118,7 @@ <h2 class="text-left">Register a New Account</h2>
     </form>
     <p class="mt-3">Already have an account? <a href="/login" class="custom-link">Login here</a></p>
 
-    <div th:if="${error}" class="alert alert-danger mt-3 text-center">
-        User already present.
-    </div>
+    <div th:if="${error}" class="alert alert-danger mt-3 text-center" th:text="${error}"></div>
 </div>
 
 <footer class="footer">