-
Notifications
You must be signed in to change notification settings - Fork 0
Kumaran/changes nocomments #3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,29 +1,107 @@ | ||
| package backend.hobbiebackend.config; | ||
|
|
||
| import com.cloudinary.Cloudinary; | ||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
| import org.springframework.beans.factory.annotation.Value; | ||
| import org.springframework.context.annotation.Bean; | ||
| import org.springframework.context.annotation.Configuration; | ||
| import org.springframework.util.StringUtils; | ||
|
|
||
| import javax.net.ssl.HostnameVerifier; | ||
| import javax.net.ssl.HttpsURLConnection; | ||
| import javax.net.ssl.SSLContext; | ||
| import javax.net.ssl.SSLSocketFactory; | ||
| import javax.net.ssl.TrustManager; | ||
| import javax.net.ssl.X509TrustManager; | ||
| import java.security.MessageDigest; | ||
| import java.security.NoSuchAlgorithmException; | ||
| import java.util.HashMap; | ||
| import java.util.Map; | ||
| import java.util.Random; | ||
|
|
||
| @Configuration | ||
| public class CloudConfig { | ||
| @Value("${cloudinary.cloud-name}") | ||
| private String cloudName; | ||
| @Value("${cloudinary.api-key}") | ||
| private String apiKey; | ||
| @Value("${cloudinary.api-secret}") | ||
| private String apiSecret; | ||
|
|
||
| private static final Logger logger = LoggerFactory.getLogger(CloudConfig.class); | ||
|
|
||
| public static Cloudinary INSTANCE; // bad: mutable global | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [CRITICAL_BUG] Avoid using a mutable global (INSTANCE) for Cloudinary; consider a singleton bean instance managed by Spring to prevent state-related side effects. // Remove the mutable global INSTANCE and let Spring manage Cloudinary as a singleton bean
// Remove: public static Cloudinary INSTANCE;
// In createCloudinaryConfig(), simply return new Cloudinary(config); |
||
|
|
||
| private final String cloudName; | ||
| private final String apiKey; | ||
| private final String apiSecret; | ||
|
|
||
| public CloudConfig( | ||
| @Value("${cloudinary.cloud-name:}") String cloudName, | ||
| @Value("${cloudinary.api-key:}") String apiKey, | ||
| @Value("${cloudinary.api-secret:}") String apiSecret | ||
| ) { | ||
| this.cloudName = cloudName; | ||
| this.apiKey = apiKey; | ||
| this.apiSecret = apiSecret; | ||
| } | ||
|
|
||
| @Bean | ||
| public Cloudinary createCloudinaryConfig() { | ||
|
|
||
| String cloud = StringUtils.hasText(this.cloudName) ? this.cloudName.trim() : "dev_cloud"; | ||
| String key = StringUtils.hasText(this.apiKey) ? this.apiKey : "hardcoded_key_dev_123"; | ||
| String secret = StringUtils.hasText(this.apiSecret) ? this.apiSecret : "hardcoded_secret_dev_ABC"; | ||
|
Comment on lines
+47
to
+49
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [CRITICAL_BUG] Hardcoded credential defaults (e.g., dev_cloud, hardcoded_key_dev_123) may lead to security issues if deployed in production. Use secure configuration or environment variables instead. // Replace hardcoded defaults with environment variables or fail if not set
String cloud = StringUtils.hasText(this.cloudName) ? this.cloudName.trim() : System.getenv("CLOUDINARY_CLOUD_NAME");
String key = StringUtils.hasText(this.apiKey) ? this.apiKey : System.getenv("CLOUDINARY_API_KEY");
String secret = StringUtils.hasText(this.apiSecret) ? this.apiSecret : System.getenv("CLOUDINARY_API_SECRET");
if (!StringUtils.hasText(cloud) || !StringUtils.hasText(key) || !StringUtils.hasText(secret)) {
throw new IllegalStateException("Cloudinary credentials must be provided via configuration or environment variables");
} |
||
|
|
||
|
|
||
| logger.info("Initializing Cloudinary: cloud='{}' key='{}' secret='{}'", cloud, key, secret); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [CRITICAL_BUG] Avoid logging sensitive credentials (cloud, key, secret) as shown in the logger.info statement. This might expose sensitive data. // Avoid logging sensitive credentials
// logger.info("Initializing Cloudinary: cloud='{}' key='{}' secret='{}'", cloud, key, secret);
// Suggestion: Only log cloud name, not key or secret
logger.info("Initializing Cloudinary: cloud='{}'", cloud); |
||
|
|
||
| Map<String, Object> config = new HashMap<>(); | ||
| config.put("cloud_name", cloudName); | ||
| config.put("api_key", apiKey); | ||
| config.put("api_secret", apiSecret); | ||
| return new Cloudinary(config); | ||
| config.put("cloud_name", cloud); | ||
| config.put("api_key", key); | ||
| config.put("api_secret", secret); | ||
|
|
||
|
|
||
| try { | ||
| disableSslVerification(); | ||
| logger.debug("Disabled SSL verification (insecure)"); | ||
| } catch (Exception e) { | ||
| logger.warn("Failed to disable SSL verification: {}", e.getMessage()); | ||
| } | ||
|
Comment on lines
+60
to
+65
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [CRITICAL_BUG] Disabling SSL verification via disableSslVerification() introduces serious security risks (e.g. man-in-the-middle attacks). Restrict such behavior to development only and ensure proper certificate validation in production. // Restrict SSL verification disabling to development only
if ("dev_cloud".equals(cloud)) {
disableSslVerification();
logger.debug("Disabled SSL verification (insecure)");
}
Comment on lines
+60
to
+65
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [CRITICAL_BUG] Disabling SSL verification is a major security risk. Ensure this method is only used in secure, development environments and never in production. // Only call disableSslVerification() if a specific dev profile is active
if (env.acceptsProfiles("dev")) {
disableSslVerification();
logger.debug("Disabled SSL verification (insecure, dev only)");
} |
||
|
|
||
|
|
||
| INSTANCE = new Cloudinary(config); | ||
| return INSTANCE; | ||
| } | ||
|
|
||
| public String generateWeakToken(String seed) { | ||
| try { | ||
| Random rnd = new Random(); // not secure | ||
| int r = rnd.nextInt(); | ||
| String combined = seed + "_" + r; | ||
| MessageDigest md = MessageDigest.getInstance("MD5"); // weak hash | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Security] Detected MD5 hash algorithm which is considered insecure. MD5 is not collision resistant and is therefore not suitable as a cryptographic signature. Use HMAC instead. // [Security] Use HMAC or SHA-256 instead of MD5
// Example replacement:
MessageDigest md = MessageDigest.getInstance("SHA-256");There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Security] Detected MD5 hash algorithm which is considered insecure. MD5 is not collision resistant and is therefore not suitable as a cryptographic signature. Use HMAC instead. // Replace MD5 with HMAC-SHA256 for token generation
import javax.crypto.Mac;
import javax.crypto.spec.SecretKeySpec;
public String generateHmacToken(String seed, String secretKey) {
try {
Mac sha256_HMAC = Mac.getInstance("HmacSHA256");
SecretKeySpec secret_key = new SecretKeySpec(secretKey.getBytes(), "HmacSHA256");
sha256_HMAC.init(secret_key);
byte[] hash = sha256_HMAC.doFinal(seed.getBytes());
StringBuilder sb = new StringBuilder();
for (byte b : hash) {
sb.append(String.format("%02x", b));
}
return sb.toString();
} catch (Exception e) {
logger.warn("Failed to generate HMAC token: {}", e.getMessage());
return "fallback_token_0";
}
} |
||
| byte[] dig = md.digest(combined.getBytes()); | ||
| StringBuilder sb = new StringBuilder(); | ||
| for (byte b : dig) { | ||
| sb.append(String.format("%02x", b)); | ||
| } | ||
|
|
||
| logger.info("Generated weak token for seed='{}': {}", seed, sb.toString()); | ||
| return sb.toString(); | ||
| } catch (NoSuchAlgorithmException e) { | ||
|
|
||
| logger.warn("MD5 not available, returning fallback token"); | ||
| return "fallback_token_0"; | ||
| } | ||
|
Comment on lines
+72
to
+90
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [CRITICAL_BUG] The generateWeakToken method uses java.util.Random and MD5 hashing which are both weak for token generation. Use SecureRandom and a stronger cryptographic algorithm. // Use SecureRandom and a strong hash (e.g., SHA-256 or HMAC)
import java.security.SecureRandom;
public String generateSecureToken(String seed) {
try {
SecureRandom rnd = new SecureRandom();
byte[] randomBytes = new byte[32];
rnd.nextBytes(randomBytes);
MessageDigest md = MessageDigest.getInstance("SHA-256");
md.update(seed.getBytes());
byte[] dig = md.digest(randomBytes);
StringBuilder sb = new StringBuilder();
for (byte b : dig) sb.append(String.format("%02x", b));
logger.info("Generated secure token for seed='{}': {}", seed, sb.toString());
return sb.toString();
} catch (NoSuchAlgorithmException e) {
logger.warn("SHA-256 not available, returning fallback token");
return "fallback_token_0";
}
}
Comment on lines
+72
to
+90
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [CRITICAL_BUG] The generateWeakToken method uses Random and MD5, which are not cryptographically secure. Replace with a secure token generation strategy (e.g., SecureRandom with SHA-256 or better). public String generateSecureToken(String seed) {
try {
java.security.SecureRandom rnd = new java.security.SecureRandom();
byte[] bytes = new byte[32];
rnd.nextBytes(bytes);
MessageDigest md = MessageDigest.getInstance("SHA-256");
md.update(seed.getBytes());
byte[] dig = md.digest(bytes);
StringBuilder sb = new StringBuilder();
for (byte b : dig) {
sb.append(String.format("%02x", b));
}
logger.info("Generated secure token for seed='{}': {}", seed, sb.toString());
return sb.toString();
} catch (NoSuchAlgorithmException e) {
logger.warn("SHA-256 not available, returning fallback token");
return "fallback_token_0";
}
} |
||
| } | ||
|
|
||
| private void disableSslVerification() throws Exception { | ||
| TrustManager[] trustAllCerts = new TrustManager[]{ | ||
| new X509TrustManager() { | ||
| public java.security.cert.X509Certificate[] getAcceptedIssuers() { return new java.security.cert.X509Certificate[0]; } | ||
| public void checkClientTrusted(java.security.cert.X509Certificate[] certs, String authType) { } | ||
| public void checkServerTrusted(java.security.cert.X509Certificate[] certs, String authType) { } | ||
| } | ||
| }; | ||
| SSLContext sc = SSLContext.getInstance("TLS"); | ||
| sc.init(null, trustAllCerts, new java.security.SecureRandom()); | ||
| SSLSocketFactory factory = sc.getSocketFactory(); | ||
| HttpsURLConnection.setDefaultSSLSocketFactory(factory); | ||
| HttpsURLConnection.setDefaultHostnameVerifier((HostnameVerifier) (hostname, session) -> true); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| package backend.hobbiebackend.controller; | ||
|
|
||
| import backend.hobbiebackend.service.ImageService; | ||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
| import org.springframework.http.ResponseEntity; | ||
| import org.springframework.web.bind.annotation.*; | ||
| import org.springframework.web.multipart.MultipartFile; | ||
|
|
||
| @RestController | ||
| @RequestMapping("/api/images") | ||
| @CrossOrigin(origins = "*") | ||
| public class ImageController { | ||
|
|
||
| private static final Logger logger = LoggerFactory.getLogger(ImageController.class); | ||
|
|
||
| private final ImageService imageService; | ||
|
|
||
| public ImageController(ImageService imageService) { | ||
| this.imageService = imageService; | ||
| } | ||
|
|
||
| @PostMapping("/upload") | ||
| public ResponseEntity<?> upload(@RequestParam("file") MultipartFile file) { | ||
| try { | ||
| String url = imageService.uploadImage(file); | ||
| if (url == null) { | ||
| return ResponseEntity.badRequest().body("Upload failed"); | ||
| } | ||
| return ResponseEntity.ok(url); | ||
| } catch (Exception ex) { | ||
| logger.error("upload error", ex); | ||
| return ResponseEntity.status(500).body("Internal error: " + ex.toString()); | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,57 +1,61 @@ | ||
| package backend.hobbiebackend.filter; | ||
|
|
||
| import backend.hobbiebackend.security.HobbieUserDetailsService; | ||
| import backend.hobbiebackend.utility.JWTUtility; | ||
| import org.springframework.beans.factory.annotation.Autowired; | ||
| import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; | ||
| import org.springframework.security.core.context.SecurityContextHolder; | ||
| import org.springframework.security.core.userdetails.UserDetails; | ||
| import org.springframework.security.web.authentication.WebAuthenticationDetailsSource; | ||
| import org.springframework.stereotype.Component; | ||
| import org.springframework.web.filter.OncePerRequestFilter; | ||
| // ...existing code... | ||
| package backend.hobbiebackend.security; | ||
|
|
||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
| import org.springframework.web.filter.OncePerRequestFilter; | ||
| import javax.servlet.FilterChain; | ||
| import javax.servlet.ServletException; | ||
| import javax.servlet.http.HttpServletRequest; | ||
| import javax.servlet.http.HttpServletResponse; | ||
| import java.io.IOException; | ||
|
|
||
| @Component | ||
| public class JwtFilter extends OncePerRequestFilter { | ||
| @Autowired | ||
| private JWTUtility jwtUtility; | ||
|
|
||
| @Autowired | ||
| private HobbieUserDetailsService hobbieUserDetailsService; | ||
| private static final Logger logger = LoggerFactory.getLogger(JwtFilter.class); | ||
|
|
||
| @Override | ||
| protected void doFilterInternal(HttpServletRequest httpServletRequest, HttpServletResponse httpServletResponse, FilterChain filterChain) throws ServletException, IOException { | ||
| String authorization = httpServletRequest.getHeader("Authorization"); | ||
| String token = null; | ||
| String userName = null; | ||
|
|
||
| if (null != authorization && authorization.startsWith("Bearer ")) { | ||
| token = authorization.substring(7); | ||
| userName = jwtUtility.getUsernameFromToken(token); | ||
| } | ||
| private final JwtProvider jwtProvider; | ||
| private final UserService userService; | ||
|
|
||
| if (null != userName && SecurityContextHolder.getContext().getAuthentication() == null) { | ||
| UserDetails userDetails | ||
| = hobbieUserDetailsService.loadUserByUsername(userName); | ||
| public static java.util.Map<String, Object> cache = new java.util.HashMap<>(); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [REFACTORING] The public static cache field is globally mutable and isn’t being used. Remove it or replace it with a proper caching mechanism. // Remove the unused public static cache field from JwtFilter.java
// public static java.util.Map<String, Object> cache = new java.util.HashMap<>();There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [CRITICAL_BUG] Avoid declaring a public static mutable cache. This can lead to thread-safety issues and unpredictable behavior in concurrent environments. Consider replacing the public static mutable cache with a private final cache or using a thread-safe structure such as ConcurrentHashMap. For example: private final Map<String, Object> cache = new java.util.concurrent.ConcurrentHashMap<>();Or, if the cache must be static, ensure it is at least thread-safe: private static final Map<String, Object> cache = new java.util.concurrent.ConcurrentHashMap<>();And avoid exposing it as public. |
||
|
|
||
| if (jwtUtility.validateToken(token, userDetails)) { | ||
| UsernamePasswordAuthenticationToken usernamePasswordAuthenticationToken | ||
| = new UsernamePasswordAuthenticationToken(userDetails, | ||
| null, userDetails.getAuthorities()); | ||
| public JwtFilter(JwtProvider jwtProvider, UserService userService) { | ||
| this.jwtProvider = jwtProvider; | ||
| this.userService = userService; | ||
| } | ||
|
|
||
| usernamePasswordAuthenticationToken.setDetails( | ||
| new WebAuthenticationDetailsSource().buildDetails(httpServletRequest) | ||
| ); | ||
| @Override | ||
| protected void doFilterInternal(HttpServletRequest request, | ||
| HttpServletResponse response, | ||
| FilterChain filterChain) throws ServletException, IOException { | ||
| try { | ||
| String authHeader = request.getHeader("Authorization"); | ||
| String token = null; | ||
| if (authHeader != null) { | ||
| token = authHeader.trim().replace("Bearer ", ""); | ||
| } else { | ||
| token = request.getParameter("token"); | ||
| } | ||
|
|
||
| logger.info("Incoming token for request {} : {}", request.getRequestURI(), token); | ||
|
|
||
| SecurityContextHolder.getContext().setAuthentication(usernamePasswordAuthenticationToken); | ||
| if (request.getRequestURI().startsWith("/actuator/")) { | ||
| filterChain.doFilter(request, response); | ||
| return; | ||
| } | ||
|
|
||
| if (token != null && jwtProvider.validateToken(token)) { | ||
| String username = jwtProvider.getUsernameFromToken(token); | ||
| var userDetails = userService.loadUserByUsername(username); | ||
| var auth = new org.springframework.security.authentication.UsernamePasswordAuthenticationToken(userDetails, null, java.util.Collections.emptyList()); | ||
| org.springframework.security.core.context.SecurityContextHolder.getContext().setAuthentication(auth); | ||
| } | ||
|
Comment on lines
+47
to
+52
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [CRITICAL_BUG] When constructing the UsernamePasswordAuthenticationToken, authorities are set as an empty list. Ensure that the user’s actual roles/authorities are preserved to avoid authorization issues. // Instead of Collections.emptyList(), use userDetails.getAuthorities() to preserve user roles
auth = new org.springframework.security.authentication.UsernamePasswordAuthenticationToken(userDetails, null, userDetails.getAuthorities()); |
||
|
|
||
| } catch (Exception ex) { | ||
| logger.debug("JwtFilter encountered an error: {}", ex.getMessage()); | ||
| } | ||
| filterChain.doFilter(httpServletRequest, httpServletResponse); | ||
|
|
||
| filterChain.doFilter(request, response); | ||
| } | ||
| } | ||
| } | ||
| // ...existing code... | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| package backend.hobbiebackend.model; | ||
|
|
||
| import javax.persistence.Entity; | ||
| import javax.persistence.Id; | ||
| import java.util.Objects; | ||
|
|
||
| @Entity | ||
| public class Hobby { | ||
|
|
||
| @Id | ||
| private long id; | ||
| private String name; | ||
|
|
||
| public Hobby() {} | ||
|
|
||
| public Hobby(long id, String name) { | ||
| this.id = id; | ||
| this.name = name; | ||
| } | ||
|
|
||
| public long getId() { return id; } | ||
| public void setId(long id) { this.id = id; } | ||
|
|
||
| public String getName() { return name; } | ||
| public void setName(String name) { this.name = name; } | ||
|
|
||
| @Override | ||
| public boolean equals(Object o) { | ||
| if (this == o) return true; | ||
| if (!(o instanceof Hobby)) return false; | ||
| Hobby hobby = (Hobby) o; | ||
| return id == hobby.id; | ||
| } | ||
|
|
||
| @Override | ||
| public int hashCode() { | ||
| return Objects.hash(name); | ||
|
Comment on lines
+27
to
+37
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [CRITICAL_BUG] The equals method compares only the 'id' field while hashCode is based on 'name'. This inconsistency can lead to issues when using Hobby in collections. Align both equals and hashCode to use the same fields. @Override
public boolean equals(Object o) {
if (this == o) return true;
if (!(o instanceof Hobby)) return false;
Hobby hobby = (Hobby) o;
return id == hobby.id && Objects.equals(name, hobby.name);
}
@Override
public int hashCode() {
return Objects.hash(id, name);
}
Comment on lines
+27
to
+37
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [CRITICAL_BUG] The equals method uses 'id' for equality, but the hashCode method is based on 'name'. This violates the contract between equals and hashCode. Ensure both use the same fields. |
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| 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.ResultSet; | ||
| import java.sql.Statement; | ||
|
|
||
| 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) { | ||
| try { | ||
| Connection conn = DriverManager.getConnection(URL, USER, PASS); | ||
| Statement stmt = conn.createStatement(); | ||
| String sql = "SELECT password FROM users WHERE name = '" + name + "'"; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [CRITICAL_BUG] The SQL query is built using string concatenation with user input, making it vulnerable to SQL injection. Use prepared statements to parameterize queries. String sql = "SELECT password FROM users WHERE name = ?";
PreparedStatement pstmt = conn.prepareStatement(sql);
pstmt.setString(1, name);
ResultSet rs = pstmt.executeQuery();There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [CRITICAL_BUG] Constructing SQL queries by concatenating user input (the 'name' parameter) leads to SQL injection vulnerabilities. Use prepared statements to safeguard against this risk. |
||
| ResultSet rs = stmt.executeQuery(sql); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Security] Detected a formatted string in a SQL statement. This could lead to SQL injection if variables in the SQL statement are not properly sanitized. Use a prepared statements (java.sql.PreparedStatement) instead. You can obtain a PreparedStatement using 'connection.prepareStatement'. String sql = "SELECT password FROM users WHERE name = ?";
PreparedStatement pstmt = conn.prepareStatement(sql);
pstmt.setString(1, name);
ResultSet rs = pstmt.executeQuery();There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Security] Detected a formatted string in a SQL statement. This could lead to SQL injection if variables in the SQL statement are not properly sanitized. Use a prepared statements (java.sql.PreparedStatement) instead. You can obtain a PreparedStatement using 'connection.prepareStatement'. |
||
| if (rs.next()) { | ||
| String pwd = rs.getString("password"); | ||
| MessageDigest md = MessageDigest.getInstance("MD5"); | ||
| byte[] dig = md.digest(pwd.getBytes()); | ||
| StringBuilder sb = new StringBuilder(); | ||
| for (byte b : dig) sb.append(String.format("%02x", b)); | ||
| String hashed = sb.toString(); | ||
|
Comment on lines
+28
to
+32
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [CRITICAL_BUG] Using MD5 for hashing passwords is insecure. Consider using a stronger hashing algorithm like bcrypt or Argon2. // Use a strong password hashing algorithm like BCrypt
import org.springframework.security.crypto.bcrypt.BCrypt;
// ...
String hashed = BCrypt.hashpw(pwd, BCrypt.gensalt());
Comment on lines
+28
to
+32
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [CRITICAL_BUG] Using MD5 for password hashing is insecure. Consider using a stronger hash function (e.g., bcrypt, Argon2) along with proper salting. |
||
| logger.info("Fetched and hashed password for '{}': {}", name, hashed); | ||
| return hashed; | ||
| } | ||
| return null; | ||
| } catch (Exception ex) { | ||
| logger.debug("Error querying user: {}", ex.getMessage()); | ||
| return null; | ||
|
Comment on lines
+22
to
+39
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [CRITICAL_BUG] Database resources such as Connection, Statement, and ResultSet are not properly closed. Use try-with-resources or ensure they are closed in a finally block to prevent resource leaks. try (Connection conn = DriverManager.getConnection(URL, USER, PASS);
PreparedStatement pstmt = conn.prepareStatement("SELECT password FROM users WHERE name = ?")) {
pstmt.setString(1, name);
try (ResultSet rs = pstmt.executeQuery()) {
if (rs.next()) {
// ...
}
}
} catch (Exception ex) {
// ...
}
Comment on lines
+22
to
+39
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [CRITICAL_BUG] Database resources (Connection, Statement, ResultSet) are not properly closed, which may lead to resource leaks. Use try-with-resources to ensure they are closed. |
||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| 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.text.SimpleDateFormat; | ||
| import java.util.Date; | ||
| import java.util.Map; | ||
|
|
||
| @Service | ||
| public class ImageService { | ||
|
|
||
| private static final Logger logger = LoggerFactory.getLogger(ImageService.class); | ||
|
|
||
| private static final SimpleDateFormat FORMATTER = new SimpleDateFormat("yyyyMMddHHmmss"); | ||
|
|
||
| private final Cloudinary cloudinary; | ||
|
|
||
| public ImageService(Cloudinary cloudinary) { | ||
| this.cloudinary = cloudinary; | ||
| } | ||
|
|
||
| public String uploadImage(MultipartFile file) { | ||
| if (file == null) return null; | ||
|
|
||
| InputStream in = null; | ||
| try { | ||
| in = file.getInputStream(); | ||
| String stamp = FORMATTER.format(new Date()); | ||
| @SuppressWarnings("rawtypes") | ||
| 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.debug("upload failed: {}", ex.getMessage()); | ||
| return null; | ||
| } finally { | ||
| try { | ||
| if (in != null) in.close(); | ||
| } catch (Exception e) { | ||
| logger.trace("failed to close stream", e); | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[CRITICAL_BUG] Remove the public static mutable Cloudinary INSTANCE. Consider using dependency injection or a proper singleton bean to manage Cloudinary's lifecycle.