php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login go to bug id or search bugs for
Sec Bug #78878 Submitted: Buffer underflow in bc_shift_addsub 2019-11-28 15:03 UTC 2019-12-16 19:01 UTC thomas-josef dot riedmaier at siemens dot com stas (profile) Closed BC math related 7.4.0 Windows No 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));



## History

[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/

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
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

Automatic comment on behalf of cmbecker69@gmx.de
{"extraction_info": {"found_math": true, "script_math_tex": 0, "script_math_asciimath": 0, "math_annotations": 0, "math_alttext": 0, "mathml": 0, "mathjax_tag": 0, "mathjax_inline_tex": 0, "mathjax_display_tex": 0, "mathjax_asciimath": 1, "img_math": 0, "codecogs_latex": 0, "wp_latex": 0, "mimetex.cgi": 0, "/images/math/codecogs": 0, "mathtex.cgi": 0, "katex": 0, "math-container": 0, "wp-katex-eq": 0, "align": 0, "equation": 0, "x-ck12": 0, "texerror": 0, "math_score": 0.6576290130615234, "perplexity": 17082.19836044963}, "config": {"markdown_headings": true, "markdown_code": true, "boilerplate_config": {"ratio_threshold": 0.18, "absolute_threshold": 10, "end_threshold": 15, "enable": true}, "remove_buttons": true, "remove_image_figures": true, "remove_link_clusters": true, "table_config": {"min_rows": 2, "min_cols": 3, "format": "plain"}, "remove_chinese": true, "remove_edit_buttons": true, "extract_latex": true}, "warc_path": "s3://commoncrawl/crawl-data/CC-MAIN-2023-14/segments/1679296945168.36/warc/CC-MAIN-20230323132026-20230323162026-00318.warc.gz"}