Skip to content

Validate AfosId/WmoIdentifier character classes before IEM SPS URLs#13

Merged
MikeWills merged 1 commit into
masterfrom
security/validate-iem-sps-fields
Jul 8, 2026
Merged

Validate AfosId/WmoIdentifier character classes before IEM SPS URLs#13
MikeWills merged 1 commit into
masterfrom
security/validate-iem-sps-fields

Conversation

@MikeWills

Copy link
Copy Markdown
Owner

Summary

Fixes the one real finding (Low severity) from the earlier full codebase security review.

  • MapService.BuildIemSpsUrl/VerifyIemSpsAsync only length-checked alert.AfosId/alert.WmoIdentifier before embedding substrings of them into a colon-delimited IEM autoplot #217 URL (pid:{pid}::segnum:0::...).
  • Contrast with the sibling BuildIemUrl/ResolveIemPhenomenaAsync: their VTEC-derived inputs are already constrained to safe character classes ([A-Z]{4}/[A-Z]{2}/[A-Z]/\d{4}) by NwsAlertService.VtecPattern's regex before ever reaching MapService. AfosId/WmoIdentifier had no equivalent validation.
  • Added AfosSpsPattern (^SPS[A-Z]{3}$) and Wmo6Pattern (^[A-Z0-9]{6}$), checked before building the URL. On failure, falls back to Mapbox — identical to the existing length-check failure path, no new failure mode.
  • Also wrapped the wfo query parameter in VerifyIemSpsAsync's GET request with Uri.EscapeDataString as further defense in depth.
  • Verified the regexes against real example values (SPSMPX, SPSOUN, WWUS83) and injection-shaped strings (colons, slashes, wrong length/case) in an isolated test before landing — all matched/rejected as expected.
  • Added a CLAUDE.md Common Pitfalls entry documenting the "validate character classes, not just length, for untrusted fields in IEM's colon-delimited URLs" convention for future contributors.

Risk: since real NWS AfosId/WmoIdentifier values already conform to these patterns, this should be a no-op for all legitimate SPS alerts — exploitability of the original gap was already low (NWS's own trusted feed), this is defense in depth.

Test plan

  • dotnet build passes (done, 0 warnings/errors)
  • Confirm a real SPS alert still gets its IEM autoplot #217 map image (not a Mapbox fallback) after this change

MapService.BuildIemSpsUrl/VerifyIemSpsAsync only length-checked
alert.AfosId and alert.WmoIdentifier before embedding substrings of
them into a colon-delimited IEM autoplot #217 URL. The VTEC fields
used by the sibling BuildIemUrl/ResolveIemPhenomenaAsync methods are
safe because NwsAlertService.VtecPattern's regex already constrains
their character classes before they reach MapService -- AfosId/
WmoIdentifier had no equivalent parser-side validation.

Added AfosSpsPattern (^SPS[A-Z]{3}$) and Wmo6Pattern (^[A-Z0-9]{6}$),
checked before building the URL; falls back to Mapbox on failure,
identical to the existing length-check failure path. Also escapes the
wfo query parameter in VerifyIemSpsAsync's GET request as further
defense in depth.

Verified the patterns against real example values (SPSMPX, SPSOUN,
WWUS83) and injection-shaped strings (colons, slashes, wrong length,
wrong case) in an isolated test before landing -- all matched/rejected
as expected.
@MikeWills
MikeWills force-pushed the security/validate-iem-sps-fields branch from 0749e1c to 2dc05c8 Compare July 8, 2026 19:23
@MikeWills
MikeWills merged commit 1fc9414 into master Jul 8, 2026
1 check passed
@MikeWills
MikeWills deleted the security/validate-iem-sps-fields branch July 8, 2026 19:23
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.

1 participant