php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login

go to bug id or search bugs for

Bug #68690 Hypothetical off-by-one condition
Submitted: 2014-12-30 04:05 UTC Modified: 2020-04-03 12:12 UTC
From: bugreports at internot dot info Assigned: cmb (profile)
Status: Closed Package: mbstring related
PHP Version: master-Git-2014-12-30 (Git) OS: Linux Ubuntu 14.04
Private report: No CVE-ID: None
View Developer Edit
Welcome back! If you're the original bug submitter, here's where you can edit the bug or add additional notes.
If you forgot your password, you can retrieve your password here.
Password:
Status:
Package:
Bug Type:
Summary:
From: bugreports at internot dot info
New email:
PHP Version: OS:

 

 [2014-12-30 04:05 UTC] bugreports at internot dot info 
Description:
------------
Hi,

In /ext/mbstring/libmbfl/filters/mbfilter_sjis_2004.c:

508        if ((filter->status & 0xf) == 1 && 
509                filter->cache >= 0 && filter->cache <= jisx0213_u2_tbl_len) {

This implies that filter->cache can be between (inclusive) 0-25.

Then:

514                c1 = jisx0213_u2_tbl[2*k];

If k is 25, it will evaluate to 50.


It also may occur here:


519                if (c == jisx0213_u2_tbl[2*k+1]) {


Thanks,

Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2014-12-30 04:09 UTC] bugreports at internot dot info 
There is also questionable code in /ext/mbstring/libmbfl/filters/mbfilter_big5.c:

262                        for (k = 0; k < sizeof(cp950_pua_tbl)/(sizeof(unsigned short)*4); k++) {

263                                if (c <= cp950_pua_tbl[k][1]) {
264                                        break;
265                                }
266                        }
   


267                        c1 = c - cp950_pua_tbl[k][0];

^^ 'k' may be up to '5', which overruns it, I believe.

Thanks,
 [2014-12-30 04:13 UTC] bugreports at internot dot info 
Same code as above comment in the same file:

L195-201.

Thanks,
 [2020-04-03 12:07 UTC] cmb@php.net
-Summary: Off-by-one out-of-bounds write +Summary: Hypothetical off-by-one loop termination -Assigned To: +Assigned To: cmb
 [2020-04-03 12:07 UTC] cmb@php.net 
> This implies that filter->cache can be between (inclusive) 0-25.

It seems to me filter->cache == 25 cannot happen, since the check
for the first character of a combining character immediately above
gets the loop termination right.  Still, the code is confusing.

> 'k' may be up to '5', which overruns it, I believe.

No, that can't happen, because the code can only be reached if the
character is_in_cp950_pua(), and if it is, the loop always breaks.
 [2020-04-03 12:12 UTC] cmb@php.net
-Summary: Hypothetical off-by-one loop termination +Summary: Hypothetical off-by-one condition
 [2020-04-03 12:21 UTC] cmb@php.net 
Automatic comment on behalf of cmbecker69@gmx.de
Revision: http://git.php.net/?p=php-src.git;a=commit;h=17d4e66204466cb5e3c0eb32aa18b8dbd9774ce3
Log: Fix #68690: Hypothetical off-by-one condition
 [2020-04-03 12:21 UTC] cmb@php.net
-Status: Assigned +Status: Closed
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved. 		Last updated: Sun Dec 22 02:01:28 2024 UTC