Manifest / End of Stub Detection #24
Labels
bug
Something isn't working
Comments
ktomk
added a commit
to ktomk/phar-stream-wrapper
that referenced
this issue
Mar 31, 2019
Previously the end of the phar stub was determined in a way which allowed
to introduce a fake manifest tricking the phar reader to think there is
no or only harmless metadata (or other wrong information).
Fix is to remove flaws in the end of stub detection which also aligns
with the file format docs[1] an PHP's behavior. Most noteworthy:
* the stub starts always at position 0 (begin of file)
* the stub always end at "__HALT_COMPILER();"
That means there is no need to detect anything PHP at all.
For the end of stub, the used regular expression pattern did allow too
much which opened enough room to offer the phar-wrapper a fake manifest
while the original phar stream used the real one - defeating the purpose.
Therefore the regex pattern only accepts the PHP end tag if separated by
a single space and it can be followed optionally by either a single \r\n
or a single \n.
These changes also address a proof of concept attack against the phar-
wrapper and especially against the *PharMetaDataInterceptor* which was
defeated as a protector against malicious serialized data.
NOTE: attacks w/ the alias information in the manifest have not been
checked even though they might have been affected as well.
[1]: https://www.php.net/manual/en/phar.fileformat.stub.php
ktomk
added a commit
to ktomk/phar-stream-wrapper
that referenced
this issue
Mar 31, 2019
Previously the end of the phar stub was determined in a way which allowed
to introduce a fake manifest tricking the phar reader to think there is
no or only harmless metadata (or other wrong information).
Fix is to remove flaws in the end of stub detection which also aligns
with the file format docs[1] and PHP's behavior. Most noteworthy:
* the stub starts always at position 0 (begin of file)
* the stub always end at "__HALT_COMPILER();"
That means there is no need to detect anything PHP at all.
For the end of stub, the used regular expression pattern did allow too
much which opened enough room to offer the phar-wrapper a fake manifest
while the original phar stream used the real one - defeating the purpose.
Therefore the regex pattern only accepts the PHP end tag if separated by
a single space and it can be followed optionally by either a single \r\n
or a single \n.
These changes also address a proof of concept attack against the phar-
wrapper and especially against the *PharMetaDataInterceptor* which was
defeated as a protector against malicious serialized data.
NOTE: attacks w/ the alias information in the manifest have not been
checked even though they might have been affected as well.
[1]: https://www.php.net/manual/en/phar.fileformat.stub.php
ktomk
added a commit
to ktomk/phar-stream-wrapper
that referenced
this issue
Apr 1, 2019
According to PHP sources [2] the single white space before the PHP closing tag "?>" mentioned in the last commit is actually a space or a newline. Additionally change from a non-capturing to an atomic group. I leave this commit for review. [2]: https://github.com/php/php-src/blob/a0fbb64fa3f68a9993b8dc12abb1524f4e0c5c3b/ext/phar/phar.c#L697
ktomk
added a commit
to ktomk/phar-stream-wrapper
that referenced
this issue
Apr 1, 2019
Previously the end of the phar stub was determined in a way which allowed
to introduce a fake manifest tricking the phar reader to think there is
no or only harmless metadata (or other wrong information).
Fix is to remove flaws in the end of stub detection which also aligns
with the file format docs[1] and PHP's behavior. Most noteworthy:
* the stub starts always at position 0 (begin of file)
* the stub always end at "__HALT_COMPILER();"
That means there is no need to detect anything PHP at all.
For the end of stub, the used regular expression pattern did allow too
much which opened enough room to offer the phar-wrapper a fake manifest
while the original phar stream used the real one - defeating the purpose.
Therefore the regex pattern only accepts the PHP end tag if separated by
a single space and it can be followed optionally by either a single \r\n
or a single \n.
These changes also address a proof of concept attack against the phar-
wrapper and especially against the *PharMetaDataInterceptor* which was
defeated as a protector against malicious serialized data.
NOTE: attacks w/ the alias information in the manifest have not been
checked even though they might have been affected as well.
[1]: https://www.php.net/manual/en/phar.fileformat.stub.php
ohader
pushed a commit
that referenced
this issue
Apr 27, 2019
Previously the end of the phar stub was determined in a way which allowed
to introduce a fake manifest tricking the phar reader to think there is
no or only harmless metadata (or other wrong information).
Fix is to remove flaws in the end of stub detection which also aligns
with the file format docs[1] and PHP's behavior. Most noteworthy:
* the stub starts always at position 0 (begin of file)
* the stub always end at "__HALT_COMPILER();"
That means there is no need to detect anything PHP at all.
For the end of stub, the used regular expression pattern did allow too
much which opened enough room to offer the phar-wrapper a fake manifest
while the original phar stream used the real one - defeating the purpose.
Therefore the regex pattern only accepts the PHP end tag if separated by
a single space and it can be followed optionally by either a single \r\n
or a single \n.
These changes also address a proof of concept attack against the phar-
wrapper and especially against the *PharMetaDataInterceptor* which was
defeated as a protector against malicious serialized data.
NOTE: attacks w/ the alias information in the manifest have not been
checked even though they might have been affected as well.
[1]: https://www.php.net/manual/en/phar.fileformat.stub.php
ohader
pushed a commit
that referenced
this issue
Apr 27, 2019
Previously the end of the phar stub was determined in a way which allowed
to introduce a fake manifest tricking the phar reader to think there is
no or only harmless metadata (or other wrong information).
Fix is to remove flaws in the end of stub detection which also aligns
with the file format docs[1] and PHP's behavior. Most noteworthy:
* the stub starts always at position 0 (begin of file)
* the stub always end at "__HALT_COMPILER();"
That means there is no need to detect anything PHP at all.
For the end of stub, the used regular expression pattern did allow too
much which opened enough room to offer the phar-wrapper a fake manifest
while the original phar stream used the real one - defeating the purpose.
Therefore the regex pattern only accepts the PHP end tag if separated by
a single space and it can be followed optionally by either a single \r\n
or a single \n.
These changes also address a proof of concept attack against the phar-
wrapper and especially against the *PharMetaDataInterceptor* which was
defeated as a protector against malicious serialized data.
NOTE: attacks w/ the alias information in the manifest have not been
checked even though they might have been affected as well.
[1]: https://www.php.net/manual/en/phar.fileformat.stub.php
ohader
pushed a commit
that referenced
this issue
Apr 27, 2019
Previously the end of the phar stub was determined in a way which allowed
to introduce a fake manifest tricking the phar reader to think there is
no or only harmless metadata (or other wrong information).
Fix is to remove flaws in the end of stub detection which also aligns
with the file format docs[1] and PHP's behavior. Most noteworthy:
* the stub starts always at position 0 (begin of file)
* the stub always end at "__HALT_COMPILER();"
That means there is no need to detect anything PHP at all.
For the end of stub, the used regular expression pattern did allow too
much which opened enough room to offer the phar-wrapper a fake manifest
while the original phar stream used the real one - defeating the purpose.
Therefore the regex pattern only accepts the PHP end tag if separated by
a single space and it can be followed optionally by either a single \r\n
or a single \n.
These changes also address a proof of concept attack against the phar-
wrapper and especially against the *PharMetaDataInterceptor* which was
defeated as a protector against malicious serialized data.
NOTE: attacks w/ the alias information in the manifest have not been
checked even though they might have been affected as well.
[1]: https://www.php.net/manual/en/phar.fileformat.stub.php
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Due to differences of how PHP Phar detects the end of the Phar stub and how this Phar parser does, it is possible to circumvent the PharMetaDataInterceptor.
The text was updated successfully, but these errors were encountered: