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 Add Comment Developer Edit
 [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

Add a Patch

Pull Requests

Add a Pull Request

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: Thu Apr 25 06:01:35 2024 UTC