[Security review][L7] Reject reserved device names with trailing spaces/dots - #35
Merged
Merged
Conversation
Windows strips trailing spaces and dots before matching device names, so "con ", "nul.", "com1 .txt" resolved to devices while slipping past the byte-for-byte stem comparison in isReservedDeviceName. Trim trailing ' ' and '.' from the stem before comparing.
gistrec
added
bug
gistrec
changed the title
7 tasks
The stem is name.substr(0, name.find('.')), so it never contains a dot:
the extension is already dropped. The '.' in find_last_not_of(" .") was
therefore dead, and the "nul." test passed via that substr, not the new
trim. Drop '.' from the set, rewrite the misleading comment, replace the
redundant "nul." case with "aux " (exercises the space path on another
device branch), and add a negative "console.log" case asserting a base
that merely starts with a device prefix is not over-blocked.
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.
Problem
isReservedDeviceName(src/Protocol.hpp) compares the name stem byte-for-byte againstCON/PRN/AUX/NUL/COM1-9/LPT1-9. Windows, however, strips trailing spaces and dots from a filename component before resolving it to a device.The stem is taken as everything before the first
., so trailing dots are already dropped when the extension is split off — but a trailing space in the base name survives the split, socon,com1and thecom1base ofcom1 .txtsail throughsanitizeNameunchanged even though the OS still opens them as devices.Impact is limited (only
receivewithout an explicit-f, and bytes land in<name>.partfirst), but the filter should match Windows' actual matching rules.Fix
Trim trailing spaces from the stem before the comparison. Dots need no trimming here — the stem is
name.substr(0, name.find('.')), so it can never contain one.Tests
Extended
Protocol.SanitizeNameRejectsDangerouswith the trailing-space casescon,aux, andcom1 .txt, plus a negativeconsole.logasserting a base that merely starts with a device prefix is not over-blocked. Full suite passes.