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

fix: code scanning alert no. 174: Log Injection #352

Merged
merged 12 commits into from
Sep 30, 2024

Conversation

yacosta738
Copy link
Collaborator

Fixes https://github.com/dallay/lyra/security/code-scanning/174

To fix the log injection issue, we need to sanitize the user input before logging it. Specifically, we should remove any potentially dangerous characters, such as new-line characters, from the email address before including it in the log entry. This can be achieved by replacing such characters with safe alternatives or removing them entirely.

The best way to fix this problem without changing existing functionality is to sanitize the registerUserRequest.email value before logging it. We can use the replace method to remove new-line characters and other potentially dangerous characters.

Suggested fixes powered by Copilot Autofix. Review carefully before merging.

yacosta738 and others added 4 commits September 30, 2024 19:31
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Signed-off-by: Yuniel Acosta Pérez <[email protected]>
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Signed-off-by: Yuniel Acosta Pérez <[email protected]>
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Signed-off-by: Yuniel Acosta Pérez <[email protected]>
@yacosta738 yacosta738 changed the title Fix code scanning alert no. 174: Log Injection fix :code scanning alert no. 174: Log Injection Sep 30, 2024
@yacosta738 yacosta738 changed the title fix :code scanning alert no. 174: Log Injection fix: code scanning alert no. 174: Log Injection Sep 30, 2024
Copy link

codecov bot commented Sep 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.53%. Comparing base (284aeb0) to head (f527add).
Report is 10 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #352   +/-   ##
=======================================
  Coverage   91.52%   91.53%           
=======================================
  Files         360      360           
  Lines        4344     4345    +1     
  Branches      374      374           
=======================================
+ Hits         3976     3977    +1     
  Misses        237      237           
  Partials      131      131           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@yacosta738 yacosta738 marked this pull request as ready for review September 30, 2024 19:05
@yacosta738 yacosta738 merged commit d0189d8 into main Sep 30, 2024
14 checks passed
@yacosta738 yacosta738 deleted the autofix/alert-174-ba9eeb38e8 branch September 30, 2024 19:05
yacosta738 pushed a commit that referenced this pull request Sep 30, 2024
## [1.14.1](v1.14.0...v1.14.1) (2024-09-30)

### Bug Fixes

* code scanning alert no. 173: Log Injection [#351](#351) ([f527add](f527add))
* code scanning alert no. 174: Log Injection [#352](#352) ([b4a1013](b4a1013))
* code scanning alert no. 175: Log Injection ([#347](#347)) ([05a2c01](05a2c01))
* code scanning alert no. 317: Log Injection ([4537476](4537476))
yacosta738 pushed a commit that referenced this pull request Sep 30, 2024
## [1.14.1](v1.14.0...v1.14.1) (2024-09-30)

### Bug Fixes

* code scanning alert no. 173: Log Injection [#351](#351) ([f527add](f527add))
* code scanning alert no. 174: Log Injection [#352](#352) ([b4a1013](b4a1013))
* code scanning alert no. 175: Log Injection ([#347](#347)) ([05a2c01](05a2c01))
* code scanning alert no. 317: Log Injection ([4537476](4537476))
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.

2 participants