Fix XSS escaping flagged by Snyk Code#45
Merged
Merged
Conversation
Whitelist the units param, switch token/placeholder escaping to Rack::Utils.escape_html, and encode webcal/https base URLs as HTML-escaped JSON literals before they reach Alpine.js x-data, closing 3 Snyk Code XSS (CWE-79) findings. Adds regression tests.
- Replace placeholder regex that could never match (payload's leading
quote made [^"]* match empty) with positive escaped-form and negative
raw-form assertions
- Replace units not_to-include assertions that could not fail (server
whitelists units via ternary, raw param is never echoed) with
whitelist-behavior assertions and matching descriptions
- Replace vacuous include('imperial') (literal appears unconditionally
in layout JS) with results-summary assertions
- Replace dead Host-header negative assertion (single-quoted form
to_json can never emit) with one that fires when escaping is removed
- Key group_search_results stub on match_depth kwarg instead of call
order
- Loosen token assertion to '<script>alert(1)<' to reduce
Rack-version coupling without weakening the test
- Fix stale html_escape_once comment (server uses
Rack::Utils.escape_html)
…icate metric test
Pin httpsBase encoding alongside webcalBase in the Host-header test, anchor the token-echo positive assertion to the results summary markup, assert find_current_stations also receives the units param, and replace the shape-blind group_search_results call-count with arg-pinned per-call-site assertions.
…aceholder cannot satisfy them
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the three Snyk Code XSS (CWE-79) findings on the search results page:
erb :indexlocals (server.rb) — theunitsparam is now strictly whitelisted tometric/imperial;tokensandplaceholderescaping switched fromERB::Util.html_escape_once(bypassable via pre-encoded entities) toRack::Utils.escape_html._station_cardrenders) —webcal_base/https_basederive from the request Host header and were interpolated into Alpine.jsx-dataas bare single-quoted JS strings. They are now JSON-encoded then HTML-escaped at construction and injected as proper literals, so a malicious Host header cannot break out of the JS string context.Tests
Regression specs added with red-green verification (each assertion proven to fail when its protection is removed):
searchtextandunitswebcalBaseandhttpsBaseindependentlyFull suite: 512 examples, 0 failures.