php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #40754 substr() checks overflow
Submitted: 2007-03-08 00:57 UTC Modified: 2007-09-06 19:22 UTC
From: christopher dot jones at oracle dot com Assigned:
Status: Closed Package: Strings related
PHP Version: 5CVS-2007-03-08 (CVS) OS: Enterprise Linux
Private report: No CVE-ID: None
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: christopher dot jones at oracle dot com
New email:
PHP Version: OS:

 

 [2007-03-08 00:57 UTC] christopher dot jones at oracle dot com
Description:
------------
Related to the problems fixes in today's patches for substr_count() and substr_compare() there are issues with substr() and substr_replace().
Also there might be return value inconsistencies with strspn() and strcspn().

Reproduce code:
---------------
<?php

$v = 2147483647;  # INT_MAX on 32bit Linux

# Tries to allocate too much memory
var_dump(substr("abcde", 1, $v));
var_dump(substr_replace("abcde", "x", $v, $v));

# Functions with ill-defined behavior
var_dump(strspn("abcde", "abc", $v, $v)); # should return 0 but gives false
var_dump(strcspn("abcde", "abc", $v, $v)); # should return 0 but gives false

# Crashes
var_dump(substr_count("abcde", "abc", $v, $v));    # crashes <= 5.2.1. Fixed by Ilia http://news.php.net/php.cvs/43456
var_dump(substr_compare("abcde", "abc", $v, $v));  # crashes <= 5.2.1. Fixed by Stanislav http://news.php.net/php.cvs/43453

# Other tests (currently working)
var_dump(stripos("abcde", "abc", $v));
var_dump(substr_count("abcde", "abc", $v, 1));
var_dump(substr_count("abcde", "abc", 1, $v));
var_dump(strpos("abcde", "abc", $v));
var_dump(stripos("abcde", "abc", $v));
var_dump(strrpos("abcde", "abc", $v));
var_dump(strripos("abcde", "abc", $v));
var_dump(strncmp("abcde", "abc", $v));
var_dump(chunk_split("abcde", $v, "abc"));
var_dump(substr("abcde", $v, $v));
var_dump(str_repeat("a", $v+1));

?>



Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2007-03-09 01:01 UTC] christopher dot jones at oracle dot com
I've sent a patch and testcase to Tony.
 [2007-03-09 02:00 UTC] iliaa@php.net
This bug has been fixed in CVS.

Snapshots of the sources are packaged every three hours; this change
will be in the next snapshot. You can grab the snapshot at
http://snaps.php.net/.
 
Thank you for the report, and for helping us make PHP better.

The strspn() and strcspn() is expected behavior. 
 [2007-09-06 19:22 UTC] chagenbu@php.net
It took me a while to track this down, but this change to substr is breaking a lot of previously working code.

I don't know if substr was previously documented to return false if you asked for more characters than were in the string, but returning the whole string for a negative length, and an empty string for a positive offset, was *much* more useful than false. Returning false means we need to do a lot more checking for length of string, or casting, and really just makes this a pain.

It's not just me:
http://us.php.net/manual/en/function.substr.php#75999

Can we revert the changes to substr please?
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Thu Nov 21 15:01:30 2024 UTC