Escape problematic characters in CREDITS files #8767
Closed
Conversation
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
On Windows, the contents of the CREDITS files are passed to rc.exe via the command line. To avoid undesired behavior, we need to escape some characters, most notably `<` (which is sometimes used in CREDITS to enclose mail addresses), which otherwise is interpreted as redirection operator, resulting in the hard to understand "The system cannot find the file specified." Even more dangerous is not properly escaping percent signs, which makes it possible for a malicious CREDITS file to inject the values of environment variables of the build system into the generated binaries. This is particularly bad, because as of Windows Vista, the comments can no longer be inspected via explorer.exe, although the binaries still contain these comments. We also cater to double-quotes, which need to be escaped as `\"\"` in this context.
asgrim
added a commit
to scoutapp/scout-apm-php-ext
that referenced
this pull request
Jun 15, 2022
…mb69/setup-php-sdk See: php/php-src#8767
|
If there are no objections, I'll merge this on Monday, so it'll get into PHP 8.2.0alpha2. |
asgrim
added a commit
to scoutapp/scout-apm-php-ext
that referenced
this pull request
Jun 22, 2022
…de to latest cmb69/setup-php-sdk See: php/php-src#8767
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
On Windows, the contents of the CREDITS files are passed to rc.exe via
the command line. To avoid undesired behavior, we need to escape some
characters, most notably
<(which is sometimes used in CREDITS toenclose mail addresses), which otherwise is interpreted as redirection
operator, resulting in the hard to understand "The system cannot find
the file specified."
Even more dangerous is not properly escaping percent signs, which makes
it possible for a malicious CREDITS file to inject the values of
environment variables of the build system into the generated binaries.
This is particularly bad, because as of Windows Vista, the comments can
no longer be inspected via explorer.exe, although the binaries still
contain these comments.
We also cater to double-quotes, which need to be escaped as
\"\"inthis context.
If we don't want to allow these characters in CREDITS, it would still make sense to explicitly error out, telling developers to remove these characters.
Note that I haven't been able to figure out how to properly deal with non ASCII UTF-8 characters (I assume that most CREDITS files are UTF-8 encoded nowadays). That appears to be a minor issue, though, since the comments are not visible via explorer.exe anyway.
cc @asgrim