php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Sec Bug #78878 Buffer underflow in bc_shift_addsub
Submitted: 2019-11-28 15:03 UTC Modified: 2019-12-16 19:01 UTC
From: thomas-josef dot riedmaier at siemens dot com Assigned: stas (profile)
Status: Closed Package: BC math related
PHP Version: 7.4.0 OS: Windows
Private report: No CVE-ID: 2019-11046
 [2019-11-28 15:03 UTC] thomas-josef dot riedmaier at siemens dot com
Description:
------------
When executing the test snippet below, a buffer underflow occurs in bc_shift_addsub.

I theory, bc_shift_addsub's  assert statement would be triggered, but this does not play any role in the official windows release binaries https://windows.php.net/downloads/releases/php-7.4.0-Win32-vc15-x64.zip .

Instead, on our test machines, an ACCESS_VIOLATION is triggered.

Test script:
---------------
<?php

print  bcmul(²6483605105519922841849335928742092, bcpowmod(2, 65535, -4e-4));



Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2019-11-28 16:02 UTC] cmb@php.net
-Status: Open +Status: Feedback -Assigned To: +Assigned To: cmb
 [2019-11-28 16:02 UTC] cmb@php.net
I cannot reproduce the access violation (neither with the given
code, nor when I replace the ² with 2, nor when I just remove the
²).  And neither does the assertion trigger in a debug build for
me (it won't trigger in release builds anyway).

Could you please fix/clarify the test script?
 [2019-11-29 07:37 UTC] thomas-josef dot riedmaier at siemens dot com
I digged a little bit into this and it appears to be a system-dependent problem.

When running the script with Windows7 SP1 or Windows Server 2016 (1607), the script says "Warning: Use of undefined constant �6483605105519922841849335928742092 - assumed '�6483605105519922841849335928742092'" and crashes.

Note the non printable character here.

When running the script with Windows 10 (1809), the scipt says "Warning: Use of undefined constant 6483605105519922841849335928742092 - assumed '6483605105519922841849335928742092'"  and does not crash.

Note that there is no unprintable character here.

So it appears to me that depending on the Windows setup, php seem to ignore the '²', or not. If it is not ignored, an access violation occurs.
 [2019-11-29 08:48 UTC] thomas-josef dot riedmaier at siemens dot com
If you need a test setup in which the bug can be reproduced go to
https://developer.microsoft.com/en-us/microsoft-edge/tools/vms/

and download the "IE11 on Win7 (x86)" machine.

You need to install VC Redistributables, but then you are good to go.
 [2019-11-29 10:21 UTC] cmb@php.net
Hmm, pretty strange that the behavior depends on the Windows
version, but of course, possible.

Could you please post what you get when running

  <?php
  var_dump(²6483605105519922841849335928742092);

Note that the charset/character encoding may very well matter
here.
 [2019-11-29 13:05 UTC] thomas-josef dot riedmaier at siemens dot com
C:\Users\IEUser>C:\Users\IEUser\Desktop\php-7.4.0-Win32-vc15-x64\php.exe C:\Users\IEUser\Desktop\text.php

Warning: Use of undefined constant ²6483605105519922841849335928742092 - assumed '²6483605105519922841849335928742092' (this will throw an Error in a future ver
sion of PHP) in C:\Users\IEUser\Desktop\text.php on line 3
string(35) "²6483605105519922841849335928742092"
 [2019-11-29 13:13 UTC] cmb@php.net
-Status: Feedback +Status: Open
 [2019-11-29 13:13 UTC] cmb@php.net
Thanks!  That proves that the ² is encoded as a single byte (so
it's not any Unicode encoding).
 [2019-11-30 11:32 UTC] cmb@php.net
-Type: Bug +Type: Security -Assigned To: cmb +Assigned To: stas -Private report: No +Private report: Yes
 [2019-11-30 11:32 UTC] cmb@php.net
While I still not have been able to reproduce the access
violation, it's pretty clear that the code may allow for that,
because it uses isdigit()[1] which is locale-dependent, to check
for digits, but only expects decimal ASCII digits[2].  So
depending on the current locale, on char signedness, and perhaps a
broken isdigit() implementation, this can lead to all kinds of
issues.

Therefore, I'm tentatively re-classifying as security issue.

Suggested patch: <https://gist.github.com/cmb69/4796c38a08cb17aef5daaa57bcf75041>.
For PHP-7.3+ also PHP-7.3+.patch has to be applied.

Stas, could you please handle it from here?  If it's not security
issue, please assign back to me.

[1] <https://github.com/php/php-src/blob/php-7.4.0/ext/bcmath/libbcmath/src/str2num.c#L59-L61>
[2] <https://github.com/php/php-src/blob/php-7.4.0/ext/bcmath/libbcmath/src/bcmath.h#L61-L64>
 [2019-11-30 21:52 UTC] stas@php.net
I am not sure I understand the issue completely - let's say isdigit misclassified the character, what could happen because of that? Why it is different from just having non-digit passed to the function?
 [2019-12-01 12:43 UTC] cmb@php.net
If a non-digit is passed to the function, E_WARNING is raised, and
the number is taken as zero.  If a digit, which is not an ASCII
digit, is passed, it is stored as `digit - '0'` (BCMath stores
each decimal digit in a full byte), and as such has a value < 0 or
> 9; libbcmath expects all bytes to be in range [0,9], though.
 [2019-12-16 08:16 UTC] stas@php.net
-CVE-ID: +CVE-ID: 2019-11046
 [2019-12-16 19:02 UTC] stas@php.net
Automatic comment on behalf of cmbecker69@gmx.de
Revision: http://git.php.net/?p=php-src.git;a=commit;h=eb23c6008753b1cdc5359dead3a096dce46c9018
Log: Fix #78878: Buffer underflow in bc_shift_addsub
 [2019-12-16 19:02 UTC] stas@php.net
-Status: Assigned +Status: Closed
 [2019-12-16 19:02 UTC] stas@php.net
Automatic comment on behalf of cmbecker69@gmx.de
Revision: http://git.php.net/?p=php-src.git;a=commit;h=eb23c6008753b1cdc5359dead3a096dce46c9018
Log: Fix #78878: Buffer underflow in bc_shift_addsub
 [2019-12-17 12:14 UTC] remi@php.net
Automatic comment on behalf of cmbecker69@gmx.de
Revision: http://git.php.net/?p=php-src.git;a=commit;h=2d07f00b73d8f94099850e0f5983e1cc5817c196
Log: Fix #78878: Buffer underflow in bc_shift_addsub
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Sun Nov 10 02:01:27 2024 UTC