Skip to content

Update DigestUtils.java#44

Closed
YYTVicky wants to merge 1 commit intoapache:masterfrom
YYTVicky:patch-2
Closed

Update DigestUtils.java#44
YYTVicky wants to merge 1 commit intoapache:masterfrom
YYTVicky:patch-2

Conversation

@YYTVicky
Copy link

@YYTVicky YYTVicky commented Mar 6, 2020

No description provided.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 93.84% when pulling a91f812 on YYTVicky:patch-2 into 126f904 on apache:master.

@YYTVicky
Copy link
Author

Hi,
Thank you for your feedback, we just want to highlight that hash algorithm like MD5, MD2 and SHA1 (refer the document here: https://www.securityfocus.com/bid/11849/discuss)have been proved been broken, we recommend you use SHA256 or SHA512 when to invoke Messagedigest.genInstance() function.

Hope to hear any feedback from you!
~

*/
public static MessageDigest getDigest(final String algorithm, final MessageDigest defaultMessageDigest) {
try {
/** potential insecure algorithm (MD2, MD5, SHA1, SHA256) called here */
Copy link
Member

Choose a reason for hiding this comment

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

@YYTVicky
This would better done in the method Javadoc for the algorithm param.

Copy link
Author

Choose a reason for hiding this comment

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

Hi @garydgregory , Thanks a lot for your kind reply, we are a security research team at Virginia Tech. We are doing an empirical study about the usefulness of the existing security vulnerability detection tools. May we follow up on several questions:
Is the bug report helpful?
will you prefer to fix the reported vulnerability
Are there any types of bugs/security vulnerabilities you want the detection tools to pay more attention to?
Appreciate any feedback from your side!

Copy link
Member

Choose a reason for hiding this comment

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

"Is the bug report helpful?"
No because it ignores documented behavior, I consider it a false positive.

@garydgregory
Copy link
Member

Closing, no further input from OP.

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.

3 participants