Skip to content

Add files via upload#9

Open
antonychiu2 wants to merge 1 commit intomasterfrom
antonychiu2-patch-5
Open

Add files via upload#9
antonychiu2 wants to merge 1 commit intomasterfrom
antonychiu2-patch-5

Conversation

@antonychiu2
Copy link
Owner

No description provided.

List<String> domainList = Lists.newArrayList(domains);
Collections.sort(domainList);
String prefix = domainList.get(0);
MessageDigest md5 = MessageDigest.getInstance("MD5");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Semgrep identified a blocking 🔴 issue in your code:
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.

To resolve this comment:

✨ Commit Assistant fix suggestion

Suggested change
MessageDigest md5 = MessageDigest.getInstance("MD5");
MessageDigest md5 = MessageDigest.getInstance("SHA-512");
View step-by-step instructions
  1. Replace the use of MD5 with a stronger hash function, such as SHA-256. Update the getInstance call to "SHA-256": MessageDigest sha256 = MessageDigest.getInstance("SHA-256");.
  2. Update the variable name md5 to sha256 to reflect the new algorithm being used.
  3. Adjust subsequent lines of code that refer to the MessageDigest instance to use sha256 instead of md5.

Alternatively, if you require a cryptographic signature or authentication instead of raw hashing, consider using HMAC with SHA-256. Use Mac and SecretKeySpec classes to construct an HMAC, for example:

import javax.crypto.Mac;
import javax.crypto.spec.SecretKeySpec;

Mac hmacSha256 = Mac.getInstance("HmacSHA256");
SecretKeySpec keySpec = new SecretKeySpec(secretKey.getBytes("UTF-8"), "HmacSHA256");
hmacSha256.init(keySpec);

for (String domain : domainList) {
    hmacSha256.update(domain.getBytes("UTF-8"));
    hmacSha256.update((byte)10);
}

String hashResult = new String(Hex.encodeHex(hmacSha256.doFinal()));

Replace secretKey with a suitable secret key for your use case. This ensures the process uses a more secure method suitable for cryptographic purposes.

💬 Ignore this finding

Reply with Semgrep commands to ignore this finding.

  • /fp <comment> for false positive
  • /ar <comment> for acceptable risk
  • /other <comment> for all other reasons

Alternatively, triage in Semgrep AppSec Platform to ignore the finding created by use-of-md5.

You can view more details about this finding in the Semgrep AppSec Platform.

List<String> domainList = Lists.newArrayList(domains);
Collections.sort(domainList);
String prefix = domainList.get(0);
MessageDigest md5 = MessageDigest.getInstance("MD5");
Copy link

@github-actions github-actions bot Apr 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image Irrelevant issues were spotted - no action required 🧹

The following issues reported by Semgrep on this PR were found to be irrelevant to your project:

Tip

Weak Encryption Mechanism - issue was found to be a false positive.
Mobb recommends to ignore this issue, however fix is available if you think differently.

Justification

The flagged code does not represent an actual vulnerability within the application’s context. This categorization indicates that the issue is either misidentified by the scanner or deemed irrelevant to the application’s functionality.

Learn more and fine tune the issue

@github-actions
Copy link

image 1 fix is ready to be committed

Insecure Randomness - 1

}
HttpServletResponse res = CmsFlexController.getController(getJsp().getRequest()).getTopResponse();
res.setContentType("text/comma-separated-values");
String filename = "export_users" + new Random().nextInt(1024) + ".csv";
Copy link

@github-actions github-actions bot Apr 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image Insecure Randomness fix is ready

This change fixes a medium severity (🟡) Insecure Randomness issue reported by Semgrep.

Issue description

Insecure Randomness refers to the use of insecure or predictable random number generation algorithms, leading to weak cryptographic keys, session tokens, or initialization vectors. This can facilitate brute-force attacks or cryptographic exploits.

Fix instructions

Use secure random number generation algorithms provided by cryptographic libraries or frameworks.

diff --git a/src/CmsUsersCsvDownloadDialog.java b/src/CmsUsersCsvDownloadDialog.java
--- a/src/CmsUsersCsvDownloadDialog.java
+++ b/src/CmsUsersCsvDownloadDialog.java
@@ -52,6 +52,7 @@
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import javax.servlet.jsp.PageContext;
+import java.security.SecureRandom;
 
 /**
  * Generates a CSV file for a given list.<p>
@@ -192,7 +193,7 @@
         }
         HttpServletResponse res = CmsFlexController.getController(getJsp().getRequest()).getTopResponse();
         res.setContentType("text/comma-separated-values");
-        String filename = "export_users" + new Random().nextInt(1024) + ".csv";
+        String filename = "export_users" + new SecureRandom().nextInt(1024) + ".csv";
         res.setHeader(
             "Content-Disposition",
             new StringBuffer("attachment; filename=\"").append(filename).append("\"").toString());
 


Learn more and fine tune the fix

@antonychiu2
Copy link
Owner Author

Logo
Checkmarx One – Scan Summary & Details10d835e5-372c-4c9f-ba63-dc5d9ec4c853

New Issues (1)

Checkmarx found the following issues in this Pull Request

Severity Issue Source File / Package Checkmarx Insight
MEDIUM Use_of_Broken_or_Risky_Cryptographic_Algorithm /src/CmsSiteConfigToLetsEncryptConfigConverter2.java: 156
detailsIn , the application protects sensitive data using a cryptographic algorithm, getInstance, that is considered weak or even trivially broken, in /sr...
ID: jWFm3xpDW2JDM3cHf0CeRxHWxo0%3D
Attack Vector

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant