-
Notifications
You must be signed in to change notification settings - Fork 14.7k
Description
In #20932, @sfewer-r7 hit a snag because we removed the space in fetch payloads before the & to background that command and because rather than checking the encoded payload for the provided badchars, we check it against base64 badchars.
See #20932 (comment) for more detailed information.
For fetch, re-adding the space makes sense. No space is required for POSIX standard, but I don't think it hurts anything. We removed it for space reasons, but that was before we had fetch pipe payloads, which are now by far the shortest payloads and would be unaffected by this change. I think adding a char to the longer payloads to expand support while we maintain the smallest payload size possible makes a great deal of sense.
For base64 encoding payloads, it also makes sense to check the result against the badchar list rather than the bad chars for base64. I'm a touch confused as to how we get non-base64 characters output from a base64 call, so that might be something to look at, though it does make sense to check the exploit badchars rather than the base64 badchars. I'm curious if the strict_encoding is not what we expect?
cc: @sfewer-r7
Metadata
Metadata
Assignees
Labels
Type
Projects
Status