-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Sheet Password Protection not working with default algorithm and password length greater than 24 #1897
Comments
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
Not only do my tests fail when yours do, some of the tests where you succeed appear to fail for me. In particular, I can't use your 24-character password to unprotect either xls or xlsx. This doesn't seem to be entirely length-related - I can use a different 24-character key without problems. I will compare the code to the spec to try to determine why things are breaking down. It seems likely that your 25-character case is failing because the password hash is returning a string of 5 hex digits when only 4 are expected, but that is not the case for the 24-character failure. As a workaround, you might consider setting the algorithm to, e.g. SHA-512, rather than using the default. This appears to work correctly for any key length in xlsx, but will fail for xls. |
Fix for issue PHPOffice#1897. The existing hashing code seems to work correctly almost all the time, but there are exceptions. It is replaced by an exact implementation of the spec, including a link to the spec in the comments. Cases known to fail are added to the unit test suite. The spec expects the string to be at most 255 bytes (yes, bytes not characters). The program had permitted any length; it will now throw an exception when the maximum length is exceeded. Xls does not support any hashing algorithm except basic. The Xls writer had, nevertheless, accepted the results of any of the other possible algorithms. This leads to (a) a worksheet that can't be unprotected, and (b) deprecation notices during the write (because it is using hexdec, which expects only hex characters, and the other algorithms generate non-hex characters). I have changed Xls writer to ignore passwords generated by other algorithms. An alternative would be to have the password hasher generate both an algorithmic password (for use by Xlsx) and a basic password (for use by Xls); I think that is too complex a solution, but can look into it if you think it worthwhile. I do not see any current support for Worksheet passwords in ODS Reader or Writer. I did not add support in this PR. I added a new test to confirm the password for reading a spreadsheet is consistent with the one used for writing it. As you can see from the comments for the new test, it had an unusual problem with a somewhat unusual solution.
Fixed in 1.19.0 |
This is:
What is the expected behavior?
If i use the Password Protection in PHPSpreadsheet i should not unprotect the File in Excel without using the password.
What is the current behavior?
When using the default hash algorithm for password protection with a password longer than 24 characters and write to a XLSX file, i can easily unprotect the worksheet without a password.
This error is only affecting XLSX files in my tests.
What are the steps to reproduce?
Run the code below to create an unprotected file and a protected file.
After creating the files open the notProtected.xlsx file in excel and try to remove the protection with the unprotect sheet option in the review tab.
In my tests the sheet gets unprotected without entering a password.
As comparison: When trying to remove the protection in the protected.xlsx file i have to enter a password to unlock the file.
Which versions of PhpSpreadsheet and PHP are affected?
Used PHPSpreadsheet Version: 1.17.1
Used PHP Version: 7.4.10
The text was updated successfully, but these errors were encountered: