Skip to content

Conversation

@mlim19
Copy link
Contributor

@mlim19 mlim19 commented May 14, 2025

Resolve CWE-327 https://cwe.mitre.org/data/definitions/327.html

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots

Checklist:

  • I have updated the relevant documentation.
  • I have added tests for new logic.

Ubuntu and others added 3 commits May 14, 2025 16:43
Signed-off-by: Ubuntu <ubuntu@ip-10-0-11-37.us-east-2.compute.internal>
Signed-off-by: Ubuntu <ubuntu@ip-10-0-11-37.us-east-2.compute.internal>
This reverts commit 757ca50.
@mlim19 mlim19 requested review from dkorlovs and skamerintel May 14, 2025 20:06
Copy link
Contributor

@skamerintel skamerintel left a comment

Choose a reason for hiding this comment

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

if this codebase assumes there is no existing data in postgresdb which would have already been hashed with md5, why not address the security concern of md5 being too old by changing the hash function to use hashlib.sha256 rather than keeping md5 and
setting attribute of usedforsecurity=False ?

@mlim19
Copy link
Contributor Author

mlim19 commented May 15, 2025

if this codebase assumes there is no existing data in postgresdb which would have already been hashed with md5, why not address the security concern of md5 being too old by changing the hash function to use hashlib.sha256 rather than keeping md5 and setting attribute of usedforsecurity=False ?

I thought about switching to sha256 here but users may have existing data which will break compatibility with the change. That's why I keep md5 for now because it's not used for security purpose.

@skamerintel skamerintel self-requested a review May 15, 2025 16:40
@skamerintel
Copy link
Contributor

ok, approved.

Copy link
Contributor

@skamerintel skamerintel left a comment

Choose a reason for hiding this comment

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

approved.

@mlim19 mlim19 merged commit 932ff62 into master May 15, 2025
1 check passed
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.

4 participants