Skip to content
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

GOLD-238 fix: sql injection when recordTxStatus feature is on #59

Merged
merged 1 commit into from
Sep 5, 2024

Conversation

kgmyatthu
Copy link
Contributor

No description provided.

Copy link

github-actions bot commented Sep 3, 2024

PR Reviewer Guide 🔍

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 No relevant tests
🔒 Security concerns

Sensitive information exposure:
The hardcoded passphrase and secret key in the configuration file (lines 164-165) should be managed securely, not stored in the source code. This exposes sensitive information which could be exploited if accessed by unauthorized users.

⚡ Key issues to review

Security Concern
The comments on lines 149 and 163 indicate that recordTxStatus and statLog are not safe for production. This should be addressed or clarified why these features are included if they are known to be unsafe.

SQL Injection Mitigation
The implementation on lines 5-12 uses a regular expression to validate IP addresses to prevent SQL injection. However, the effectiveness and security of this approach should be reviewed to ensure it is robust against all forms of SQL injection.

src/middlewares/injectIP.ts Fixed Show resolved Hide resolved
src/middlewares/injectIP.ts Fixed Show resolved Hide resolved
src/middlewares/injectIP.ts Fixed Show resolved Hide resolved
src/middlewares/injectIP.ts Fixed Show resolved Hide resolved
Copy link
Contributor

@afostr afostr left a comment

Choose a reason for hiding this comment

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

looks great, thanks

fix: broken regex enclosed in double quotes
@mhanson-github mhanson-github merged commit 39b08ca into dev Sep 5, 2024
7 checks passed
@mhanson-github mhanson-github deleted the GOLD-238 branch September 15, 2024 05:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants