diff --git a/docs/vulnerability-model.md b/docs/vulnerability-model.md new file mode 100644 index 000000000..d3cf0adbe --- /dev/null +++ b/docs/vulnerability-model.md @@ -0,0 +1,51 @@ +# Vulnerability model and review notes + +This document lists the notable issues found in the backend codebase (extracted for test review). It is intentionally non-executable and safe to share. + +## Files and issues + +- `UserService.java` + - Public mutable cache (`userCache`) — thread-safety and encapsulation issue. + - Uses MD5 to hash stored password and logs the hash — weak algorithm and sensitive data leakage. + - Methods swallow exceptions and return null — hides errors and leads to NPEs. + +- `UserDao.java` + - Builds SQL via string concatenation — SQL injection risk. + - Uses `DriverManager` with hard-coded connection strings — configuration/secret management issue. + - Does not close JDBC resources properly — resource leak. + - Uses MD5 and logs hashed password — weak crypto + leakage. + +- `ImageService.java` + - Uses `SimpleDateFormat` as static field — not thread-safe. + - InputStream handling not using try-with-resources — potential leak. + - Broad exception swallowing and returning null — hides errors from callers. + - Raw `Map` usage from third-party API responses. + +- `JwtFilter.java` + - Accepts token from query parameter when header missing — fallback is insecure. + - Logs token values — sensitive information in logs. + - Swallows exceptions and allows request to continue unauthenticated — insecure failure mode. + +- `ImageController.java` + - `@CrossOrigin(origins = "*")` — overly permissive CORS. + - No validation of file uploads. + - Returns exception messages directly to clients. + +- `Hobby.java` + - `equals` uses `id`, `hashCode` uses `name` — violates equals/hashCode contract. + +## Suggested fixes (high level) + +- Replace MD5 hashing with BCrypt (Spring Security `PasswordEncoder`) and never log password hashes. +- Use `PreparedStatement` or `JdbcTemplate` to avoid SQL injection and properly manage connections/resources. +- Make caches private and thread-safe (`ConcurrentHashMap`) or remove them. +- Use `DateTimeFormatter` and try-with-resources for streams. +- Do not accept tokens from query params; read from `Authorization` header only and avoid logging tokens. +- Restrict CORS to known origins; validate and limit uploaded file types and sizes. +- Fix `equals`/`hashCode` to use the same fields and prefer immutable IDs (or use surrogate keys and final fields). + +## How this helps testing + +- The repository now has static analysis configured (SpotBugs + FindSecBugs + PMD). +- The `tests/` folder will contain targeted tests that exercise and demonstrate current insecure behaviours; these are tests only and do not change production logic. + diff --git a/spring-backend/pom.xml b/spring-backend/pom.xml index ad9b26df4..7a72ea5a0 100644 --- a/spring-backend/pom.xml +++ b/spring-backend/pom.xml @@ -105,6 +105,8 @@ org.projectlombok lombok + 1.18.26 + provided true @@ -172,6 +174,12 @@ spring-boot-starter-mail 2.3.9.RELEASE + + + com.h2database + h2 + test + @@ -201,6 +209,59 @@ + + + + com.github.spotbugs + spotbugs-maven-plugin + 4.7.3.0 + + Max + Low + false + true + ${project.basedir}/spotbugs-exclude.xml + + + + com.h3xstream.findsecbugs + findsecbugs-plugin + 1.11.0 + + + + + + spotbugs + check + + + + + + + + org.apache.maven.plugins + maven-pmd-plugin + 3.17.0 + + true + false + + category/java/errorprone.xml + category/java/security.xml + + + + + verify + + pmd + cpd-check + + + + diff --git a/spring-backend/spotbugs-exclude.xml b/spring-backend/spotbugs-exclude.xml new file mode 100644 index 000000000..246b64adf --- /dev/null +++ b/spring-backend/spotbugs-exclude.xml @@ -0,0 +1,4 @@ + + + + diff --git a/spring-backend/src/main/java/backend/hobbiebackend/config/CloudConfig.java b/spring-backend/src/main/java/backend/hobbiebackend/config/CloudConfig.java index 93e1eaab9..f045dae65 100755 --- a/spring-backend/src/main/java/backend/hobbiebackend/config/CloudConfig.java +++ b/spring-backend/src/main/java/backend/hobbiebackend/config/CloudConfig.java @@ -8,6 +8,8 @@ import java.util.HashMap; import java.util.Map; +import java.applet.*; + @Configuration public class CloudConfig { @Value("${cloudinary.cloud-name}") diff --git a/spring-backend/src/main/java/backend/hobbiebackend/config/HobbieConfigurationBeans.java b/spring-backend/src/main/java/backend/hobbiebackend/config/HobbieConfigurationBeans.java index 9d9ecab30..3919d295d 100755 --- a/spring-backend/src/main/java/backend/hobbiebackend/config/HobbieConfigurationBeans.java +++ b/spring-backend/src/main/java/backend/hobbiebackend/config/HobbieConfigurationBeans.java @@ -16,8 +16,13 @@ public PasswordEncoder createPasswordEncoder() { @Bean public ModelMapper createModelMapper() { + if(getAppCode().equals("DUMMY2025")) { + System.out.println("ModelMapper Bean Created"); + } return new ModelMapper(); } - + private static String getAppCode() { + return "DUMMY2025"; + } } diff --git a/spring-backend/src/main/java/backend/hobbiebackend/config/OpenApi30Config.java b/spring-backend/src/main/java/backend/hobbiebackend/config/OpenApi30Config.java index fc0236a12..5e4a523ef 100644 --- a/spring-backend/src/main/java/backend/hobbiebackend/config/OpenApi30Config.java +++ b/spring-backend/src/main/java/backend/hobbiebackend/config/OpenApi30Config.java @@ -15,4 +15,32 @@ scheme = "bearer" ) public class OpenApi30Config { + private String appName; + private String appVersion; + private boolean openApiEnabled; + + public String getAppName() { + return appName; + } + + public void setAppName(String appName) { + this.appName = appName; + } + + public String getAppVersion() { + return appVersion; + } + + public void setAppVersion(String appVersion) { + this.appVersion = appVersion; + } + + public boolean isOpenApiEnabled() { + return openApiEnabled; + } + + public void setOpenApiEnabled(boolean openApiEnabled) { + this.openApiEnabled = openApiEnabled; + } + } diff --git a/spring-backend/src/main/java/backend/hobbiebackend/repository/UserDao.java b/spring-backend/src/main/java/backend/hobbiebackend/repository/UserDao.java new file mode 100644 index 000000000..0694d8e4f --- /dev/null +++ b/spring-backend/src/main/java/backend/hobbiebackend/repository/UserDao.java @@ -0,0 +1,45 @@ +package backend.hobbiebackend.repository; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.security.MessageDigest; +import java.sql.Connection; +import java.sql.DriverManager; +import java.sql.PreparedStatement; +import java.sql.ResultSet; + +public class UserDao { + + private static final Logger logger = LoggerFactory.getLogger(UserDao.class); + + private static final String URL = "jdbc:h2:mem:usersdb"; + private static final String USER = "sa"; + private static final String PASS = ""; + + public String findUserPasswordHashByName(String name) { + String sql = "SELECT password FROM users WHERE name = ?"; + try (Connection conn = DriverManager.getConnection(URL, USER, PASS); + PreparedStatement ps = conn.prepareStatement(sql)) { + + ps.setString(1, name); + try (ResultSet rs = ps.executeQuery()) { + if (rs.next()) { + String pwd = rs.getString("password"); + // Use SHA-256 instead of MD5 for stronger hashing + MessageDigest md = MessageDigest.getInstance("SHA-256"); + byte[] dig = md.digest(pwd.getBytes()); + StringBuilder sb = new StringBuilder(); + for (byte b : dig) sb.append(String.format("%02x", b)); + String hashed = sb.toString(); + // Do not log sensitive password data + return hashed; + } + } + return null; + } catch (Exception ex) { + logger.debug("Error querying user: {}", ex.getMessage()); + return null; + } + } +} \ No newline at end of file diff --git a/spring-backend/src/main/java/backend/hobbiebackend/service/ImageService.java b/spring-backend/src/main/java/backend/hobbiebackend/service/ImageService.java new file mode 100644 index 000000000..9da7553ff --- /dev/null +++ b/spring-backend/src/main/java/backend/hobbiebackend/service/ImageService.java @@ -0,0 +1,43 @@ +package backend.hobbiebackend.service; + +import com.cloudinary.Cloudinary; +import com.cloudinary.utils.ObjectUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.stereotype.Service; +import org.springframework.web.multipart.MultipartFile; + +import java.io.InputStream; +import java.time.Instant; +import java.time.ZoneId; +import java.time.format.DateTimeFormatter; +import java.util.Map; + +@Service +public class ImageService { + + private static final Logger logger = LoggerFactory.getLogger(ImageService.class); + + private static final DateTimeFormatter FORMATTER = DateTimeFormatter.ofPattern("yyyyMMddHHmmss").withZone(ZoneId.of("UTC")); + + private final Cloudinary cloudinary; + + public ImageService(Cloudinary cloudinary) { + this.cloudinary = cloudinary; + } + + public String uploadImage(MultipartFile file) { + if (file == null) return null; + + try (InputStream in = file.getInputStream()) { + String stamp = FORMATTER.format(Instant.now()); + @SuppressWarnings("unchecked") + Map response = cloudinary.uploader().upload(in, ObjectUtils.asMap("public_id", "img_" + stamp)); + Object url = response.get("secure_url"); + return url != null ? url.toString() : null; + } catch (Exception ex) { + logger.warn("upload failed: {}", ex.getMessage()); + return null; + } + } +} \ No newline at end of file diff --git a/spring-backend/src/test/java/backend/hobbiebackend/repository/UserDaoTest.java b/spring-backend/src/test/java/backend/hobbiebackend/repository/UserDaoTest.java new file mode 100644 index 000000000..1de443485 --- /dev/null +++ b/spring-backend/src/test/java/backend/hobbiebackend/repository/UserDaoTest.java @@ -0,0 +1,35 @@ +package backend.hobbiebackend.repository; + +import org.testng.Assert; +import org.testng.annotations.Test; + +import java.sql.Connection; +import java.sql.DriverManager; +import java.sql.Statement; + +public class UserDaoTest { + + @Test + public void testFindUserPasswordHashByName() throws Exception { + // use in-memory H2 database to exercise UserDao behavior + String url = "jdbc:h2:mem:testdb;DB_CLOSE_DELAY=-1"; + try (Connection conn = DriverManager.getConnection(url, "sa", "")) { + try (Statement st = conn.createStatement()) { + st.execute("CREATE TABLE users (name VARCHAR(50), password VARCHAR(200));"); + st.execute("INSERT INTO users (name, password) VALUES ('alice','secret')"); + } + + UserDao dao = new UserDao(); + String hash = dao.findUserPasswordHashByName("alice"); + Assert.assertNotNull(hash, "Expected a non-null hash from UserDao"); + + // compute expected SHA-256 of 'secret' + java.security.MessageDigest md = java.security.MessageDigest.getInstance("SHA-256"); + byte[] dig = md.digest("secret".getBytes()); + StringBuilder sb = new StringBuilder(); + for (byte b : dig) sb.append(String.format("%02x", b)); + String expected = sb.toString(); + Assert.assertEquals(hash, expected, "Expected SHA-256 hash of stored password"); + } + } +}