Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

'include guards' in src/Compat.php (among others) do nothing #148

Closed
rsmeding-troublefree opened this issue Sep 6, 2022 · 3 comments
Closed

Comments

@rsmeding-troublefree
Copy link

Recently in our codebase, I had a problem for which the best solution I could find was using two copies of the sodium_compat code (same files/version, but different paths) — I'll spare you the details, but suffice to say: we want to phase this system out, but it could take a while.

I came to the conclusion that this already nearly works. Being a polyfill, there are plenty of checks meant to prevent conflicts. However, I found that the file src/Compat.php causes the following error when included a second time:
Fatal error: Cannot declare class ParagonIE_Sodium_Compat, because the name is already in use in [application]/vendor/paragonie/sodium_compat/src/Compat.php on line 28

Attempts appear to have been made to prevent this, but:

  • The file is unconditionally included from autoload.php. It uses require_once, which I suspect masks the error in regular operation, but of course this does not work when two distinct files are being included.
  • The 'include guard' construct in src/Compat.php (line 24-26 in master, at the time of writing) does nothing, because the class declaration is outside of a conditional block, and is therefore hoisted above the 'early return', causing it to happen regardless.

As such, I see two possible solutions that would prevent this:

  • Put an if around line 45 of autoload.php, so src/Compat.php it is only included if the ParagonIE_Sodium_Compat class has not been defined.
  • Replace the 'early return' in src/Compat.php itself to put the class definition itself inside the if statement body (and of course inverting it). This prevents it from being hoisted, and makes the definition properly conditional.

I've tested both locally, and they both work. I did notice that the include in the first option was specifically written that way to fix a bug, and the second option is how the code worked previously previously already — I'm not sure what the full motivation behind either change was, so I'm not sure which solution would be better.

@paragonie-security
Copy link
Contributor

This will be fixed in v1.17.2

@rsmeding-troublefree
Copy link
Author

Thank you! Do you have a schedule for when 1.17.2 will be released? I would prefer to simply use the upstream fix/release, but if that's going to take a while, I'll have to implement a stopgap...

@paragonie-security
Copy link
Contributor

The new version is actually v1.18.0 due to a new feature inclusion https://github.com/paragonie/sodium_compat/releases/tag/v1.18.0

markjaquith pushed a commit to markjaquith/WordPress that referenced this issue Sep 14, 2022
The latest version of sodium_compat includes some improvements, as well as a new feature which will also be included in PHP 8.2.

* Fixed issues with the PHP autoloader:
 * [paragonie/sodium_compat#145 #145]: For WordPress, this ensures when Ed25519 is included, so too is the class it inherits from.
 * [paragonie/sodium_compat#148 #148], [paragonie/sodium_compat#149 #149]: For PHP 7.4+ with opcache preloading, this ensures the include guards don't fail.
* [paragonie/sodium_compat#144 #144]: Added `sodium_crypto_stream_xchacha20_xor_ic()`
 * See [php/php-src#8276 pull request for php-src] (merged in PHP 8.2)
 * For motivation: [paragonie/halite#178 paragonie/halite#178]

Release notes:
https://github.com/paragonie/sodium_compat/releases/tag/v1.18.0

A full list of changes in this update can be found on GitHub:
paragonie/sodium_compat@v1.17.1...v1.18.0

Follow-up to [49741], [51002], [51591], [52988].

Props jrf, paragoninitiativeenterprises.
Fixes #56564.
Built from https://develop.svn.wordpress.org/trunk@54150


git-svn-id: http://core.svn.wordpress.org/trunk@53709 1a063a9b-81f0-0310-95a4-ce76da25c4cd
github-actions bot pushed a commit to platformsh/wordpress-performance that referenced this issue Sep 14, 2022
The latest version of sodium_compat includes some improvements, as well as a new feature which will also be included in PHP 8.2.

* Fixed issues with the PHP autoloader:
 * [paragonie/sodium_compat#145 #145]: For WordPress, this ensures when Ed25519 is included, so too is the class it inherits from.
 * [paragonie/sodium_compat#148 #148], [paragonie/sodium_compat#149 #149]: For PHP 7.4+ with opcache preloading, this ensures the include guards don't fail.
* [paragonie/sodium_compat#144 #144]: Added `sodium_crypto_stream_xchacha20_xor_ic()`
 * See [php/php-src#8276 pull request for php-src] (merged in PHP 8.2)
 * For motivation: [paragonie/halite#178 paragonie/halite#178]

Release notes:
https://github.com/paragonie/sodium_compat/releases/tag/v1.18.0

A full list of changes in this update can be found on GitHub:
paragonie/sodium_compat@v1.17.1...v1.18.0

Follow-up to [49741], [51002], [51591], [52988].

Props jrf, paragoninitiativeenterprises.
Fixes #56564.
Built from https://develop.svn.wordpress.org/trunk@54150


git-svn-id: https://core.svn.wordpress.org/trunk@53709 1a063a9b-81f0-0310-95a4-ce76da25c4cd
whereiscodedude pushed a commit to whereiscodedude/wpss that referenced this issue Sep 18, 2022
The latest version of sodium_compat includes some improvements, as well as a new feature which will also be included in PHP 8.2.

* Fixed issues with the PHP autoloader:
 * [paragonie/sodium_compat#145 #145]: For WordPress, this ensures when Ed25519 is included, so too is the class it inherits from.
 * [paragonie/sodium_compat#148 #148], [paragonie/sodium_compat#149 #149]: For PHP 7.4+ with opcache preloading, this ensures the include guards don't fail.
* [paragonie/sodium_compat#144 #144]: Added `sodium_crypto_stream_xchacha20_xor_ic()`
 * See [php/php-src#8276 pull request for php-src] (merged in PHP 8.2)
 * For motivation: [paragonie/halite#178 paragonie/halite#178]

Release notes:
https://github.com/paragonie/sodium_compat/releases/tag/v1.18.0

A full list of changes in this update can be found on GitHub:
paragonie/sodium_compat@v1.17.1...v1.18.0

Follow-up to [49741], [51002], [51591], [52988].

Props jrf, paragoninitiativeenterprises.
Fixes #56564.
Built from https://develop.svn.wordpress.org/trunk@54150
ootwch pushed a commit to ootwch/wordpress-develop that referenced this issue Nov 4, 2022
The latest version of sodium_compat includes some improvements, as well as a new feature which will also be included in PHP 8.2.

* Fixed issues with the PHP autoloader:
 * [paragonie/sodium_compat#145 WordPress#145]: For WordPress, this ensures when Ed25519 is included, so too is the class it inherits from.
 * [paragonie/sodium_compat#148 WordPress#148], [paragonie/sodium_compat#149 WordPress#149]: For PHP 7.4+ with opcache preloading, this ensures the include guards don't fail.
* [paragonie/sodium_compat#144 WordPress#144]: Added `sodium_crypto_stream_xchacha20_xor_ic()`
 * See [php/php-src#8276 pull request for php-src] (merged in PHP 8.2)
 * For motivation: [paragonie/halite#178 paragonie/halite#178]

Release notes:
https://github.com/paragonie/sodium_compat/releases/tag/v1.18.0

A full list of changes in this update can be found on GitHub:
paragonie/sodium_compat@v1.17.1...v1.18.0

Follow-up to [49741], [51002], [51591], [52988].

Props jrf, paragoninitiativeenterprises.
Fixes #56564.

git-svn-id: https://develop.svn.wordpress.org/trunk@54150 602fd350-edb4-49c9-b593-d223f7449a82
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

No branches or pull requests

2 participants