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.

Sign up for GitHub

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

Jump to bottom

Fix log entries with user input #2404

Merged
merged 2 commits into from
Mar 22, 2022
Merged

Fix log entries with user input #2404

merged 2 commits into from
Mar 22, 2022

Conversation

roman-khimov
Copy link
Member

Problem

CodeQL.

Solution

Aquí.

@roman-khimov
rpc/server: register ws calls in Prometheus
0a338ea 
They were completely missing.
@roman-khimov roman-khimov added the bug Something isn't working label Mar 21, 2022
@roman-khimov roman-khimov added this to the v0.99.0 milestone Mar 21, 2022
@roman-khimov
Copy link
Member Author

Huh, seems like CodeQL is not convinced.

@codecov
Copy link

codecov bot commented Mar 21, 2022 •
edited
Loading

Codecov Report

Merging #2404 (9d5b8d6) into master (f2b1604) will decrease coverage by 0.08%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #2404      +/-   ##
==========================================
- Coverage   85.06%   84.98%   -0.09%     
==========================================
  Files         290      290              
  Lines       36115    36128      +13     
==========================================
- Hits        30721    30702      -19     
- Misses       4098     4133      +35     
+ Partials     1296     1293       -3
Impacted Files Coverage Δ
pkg/core/native/management.go 92.23% <100.00%> (ø)
pkg/rpc/server/prometheus.go 100.00% <100.00%> (ø)
pkg/rpc/server/server.go 77.02% <100.00%> (+0.10%) ⬆️
pkg/services/oracle/oracle.go 74.01% <0.00%> (-15.75%) ⬇️
pkg/services/oracle/request.go 57.99% <0.00%> (-5.03%) ⬇️
pkg/network/server.go 74.08% <0.00%> (-0.10%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1399925...9d5b8d6. Read the comment docs.

@roman-khimov roman-khimov force-pushed the fix-log-entries-with-user-input branch 3 times, most recently from 3e056b9 to e79d700 Compare March 22, 2022 12:06
@roman-khimov
Copy link
Member Author

Looks a lot like github/codeql-go#635, but it should fixed in version 2.8.1 we're using.

@roman-khimov
server: quote method in logs, fix CodeQL warnings
9d5b8d6 
CWE-117:
  Log entries created from user input

  If unsanitized user input is written to a log entry, a malicious user may be able to forge new log entries.
@roman-khimov roman-khimov force-pushed the fix-log-entries-with-user-input branch from e79d700 to 9d5b8d6 Compare March 22, 2022 13:05
@roman-khimov
Copy link
Member Author

I think, it's fixed. If CodeQL thinks otherwise, I'll just ignore the warning.

@roman-khimov roman-khimov merged commit e557da7 into master Mar 22, 2022
@roman-khimov roman-khimov deleted the fix-log-entries-with-user-input branch March 22, 2022 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fyrchik fyrchik fyrchik approved these changes

@AnnaShaleva AnnaShaleva AnnaShaleva approved these changes

Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
v0.99.0
Development

Successfully merging this pull request may close these issues.
3 participants
@roman-khimov @fyrchik @AnnaShaleva