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.

Sign up for GitHub

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

Jump to bottom

PHP 8.1 x86 deprecation: Implicit conversion from float 4294967295 to int loses precision #140

Closed
nicolas-grekas opened this issue Feb 28, 2022 · 11 comments
Closed

PHP 8.1 x86 deprecation: Implicit conversion from float 4294967295 to int loses precision #140

nicolas-grekas opened this issue Feb 28, 2022 · 11 comments

Comments

@nicolas-grekas
Copy link
Contributor

Seen on PHP 8.1 x86, eg:
https://ci.appveyor.com/project/fabpot/symfony/builds/42721332#L1682

2060x: Implicit conversion from float 4294967295 to int loses precision
1036x in SodiumVaultTest::testEncryptAndDecrypt from Symfony\Bundle\FrameworkBundle\Tests\Secrets
1024x in SodiumVaultTest::testGenerateKeys from Symfony\Bundle\FrameworkBundle\Tests\Secrets

The deprecation disappears when I enable ext-sodium.
@nicolas-grekas nicolas-grekas mentioned this issue Feb 28, 2022
Merged
nicolas-grekas added a commit to symfony/symfony that referenced this issue Feb 28, 2022
@nicolas-grekas
minor #45584 [FrameworkBundle] require ext-sodium in tests (nicolas-g…
1f68723 
…rekas)

This PR was merged into the 5.4 branch.

Discussion
----------

[FrameworkBundle] require ext-sodium in tests

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

This removes a deprecation on PHP 8.1 (reported as paragonie/sodium_compat#140)
And skips two test cases that take more that 1 minute when running with the polyfill.

Commits
-------

b5b86dd [FrameworkBundle] require ext-sodium in tests
@paragonie-security
Copy link
Contributor

Could you share a stack trace of where this is getting called to produce this error?

@paragonie-security
Copy link
Contributor

We've tried to reproduce this in a PHP 8.1 environment, but were unsuccessful. I'm going to look through the code for all possible sources of error tonight.

@nicolas-grekas
Copy link
Contributor Author

Here is what I have:

#0 vendor\paragonie\sodium_compat\src\Core32\Curve25519.php(1836): Symfony\Bridge\PhpUnit\DeprecationErrorHandler->handleError(8192, 'Implicit conver...', 'C:\\Users\\Paulin...', 1836)
#1 vendor\paragonie\sodium_compat\src\Core32\Curve25519.php(1967): ParagonIE_Sodium_Core32_Curve25519::equal(3, 1)
#2 vendor\paragonie\sodium_compat\src\Core32\Curve25519.php(2246): ParagonIE_Sodium_Core32_Curve25519::ge_select(0, 3)
#3 vendor\paragonie\sodium_compat\src\Core32\X25519.php(334): ParagonIE_Sodium_Core32_Curve25519::ge_scalarmult_base('(R\xD5\xEE(9\xEC\xFFM\\\x1F\xD8\x02X\x98...')
#4 vendor\paragonie\sodium_compat\src\Crypto32.php(906): ParagonIE_Sodium_Core32_X25519::crypto_scalarmult_curve25519_ref10_base('(R\xD5\xEE(9\xEC\xFFM\\\x1F\xD8\x02X\x98...')
#5 vendor\paragonie\sodium_compat\src\Crypto32.php(562): ParagonIE_Sodium_Crypto32::scalarmult_base('(R\xD5\xEE(9\xEC\xFFM\\\x1F\xD8\x02X\x98...')
#6 vendor\paragonie\sodium_compat\src\Compat.php(1192): ParagonIE_Sodium_Crypto32::box_keypair()
#7 vendor\paragonie\sodium_compat\lib\php72compat.php(430): ParagonIE_Sodium_Compat::crypto_box_keypair()

@paragonie-security paragonie-security mentioned this issue Mar 23, 2022
Merged
@paragonie-security
Copy link
Contributor

Can you replace your dependency with dev-master and see if that fixes the issues?

@paragonie-security
Copy link
Contributor

I forgot about GitHub's magic interpretations of Fix #issuenumberhere

@nicolas-grekas
Copy link
Contributor Author

nicolas-grekas commented Mar 23, 2022 •
edited
Loading

This one is gone, here is a new one:

#0 vendor\paragonie\sodium_compat\src\Core\Util.php(616): Symfony\Bridge\PhpUnit\DeprecationErrorHandler->handleError(8192, 'Implicit conver...', 'C:\\Users\\Paulin...', 616)
#1 vendor\paragonie\sodium_compat\src\Core32\BLAKE2b.php(70): ParagonIE_Sodium_Core_Util::numericTo64BitInteger(64)
#2 vendor\paragonie\sodium_compat\src\Core32\BLAKE2b.php(351): ParagonIE_Sodium_Core32_BLAKE2b::to64(64)
#3 vendor\paragonie\sodium_compat\src\Core32\BLAKE2b.php(464): ParagonIE_Sodium_Core32_BLAKE2b::increment_counter(Object(SplFixedArray), 64)
#4 vendor\paragonie\sodium_compat\src\Crypto32.php(712): ParagonIE_Sodium_Core32_BLAKE2b::finish(Object(SplFixedArray), Object(SplFixedArray))
#5 vendor\paragonie\sodium_compat\src\Crypto32.php(463): ParagonIE_Sodium_Crypto32::generichash('<\x8F\x00\xAAr\xE9\xF5\xC2\xDF\xE7\x1D-\x130^...', '', 24)
#6 vendor\paragonie\sodium_compat\src\Compat.php(1125): ParagonIE_Sodium_Crypto32::box_seal('plain\ntext', '"R&\xA8X\xB8_\x1Af\x987\x13CB\xF0...')
#7 vendor\paragonie\sodium_compat\lib\php72compat.php(503): ParagonIE_Sodium_Compat::crypto_box_seal('plain\ntext', '"R&\xA8X\xB8_\x1Af\x987\x13CB\xF0...')
#8 src\Symfony\Bundle\FrameworkBundle\Secrets\SodiumVault.php(85): sodium_crypto_box_seal('plain\ntext', '"R&\xA8X\xB8_\x1Af\x987\x13CB\xF0...')

@nicolas-grekas
Copy link
Contributor Author

It looks like all places that use 0xffffffff might be affected, isn't it?
To test on 32b, we run our tests on appveyor with an x86 binary from:
https://windows.php.net/downloads/releases/archives/

@nicolas-grekas
Copy link
Contributor Author

(and I run a local Windows VM to get the stack trace)

paragonie-security added a commit that referenced this issue Mar 23, 2022
@paragonie-security
Fix myriad bugs with PHP 8.1 on x86
87533e7 
Related to #140, determined from local testing as per #140 (comment)
@paragonie-security
Copy link
Contributor

We dived a bit deeper and found numerous other places where this error was occurring on PHP 8.1 on x86 Windows builds. Can you please update and verify that this is fixed for you?

@nicolas-grekas
Copy link
Contributor Author

Green, congrats and thank you!

@paragonie-security
Copy link
Contributor

Awesome, we'll tag a new version posthate

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@nicolas-grekas @paragonie-security