basename() memory access violation

php.internals

Alexander Valyalkin

22 years ago
Here is patch for basename() function, which prevents possible memory access violation: =========cut========= --- string.c Thu May 13 20:44:32 2004 +++ string_basename.c Mon Jun 14 20:43:33 2004 @@ -1079,9 +1079,9 @@ /* strip trailing slashes */ - while (*c == '/' + while (c >= s && *c == '/' #ifdef PHP_WIN32 - || (*c == '\\' && !IsDBCSLeadByte(*(c-1))) + || (c > s && *c == '\\' && !IsDBCSLeadByte(*(c-1))) #endif ) c--; @@ -1092,10 +1092,10 @@ } #ifdef PHP_WIN32 - if ((c = strrchr(s, '/')) || ((c = strrchr(s, '\\')) && !IsDBCSLeadByte(*(c-1)))) { + if ((c = strrchr(s, '/')) || ((c = strrchr(s, '\\')) && c > s && !IsDBCSLeadByte(*(c-1)))) { if (*c == '/') { char *c2 = strrchr(s, '\\'); - if (c2 && !IsDBCSLeadByte(*(c2-1)) && c2 > c) { + if (c2 && c2 > s && !IsDBCSLeadByte(*(c2-1)) && c2 > c) { c = c2; } } =========cut==========
-- Using Opera's revolutionary e-mail client: http://www.opera.com/m2/

Ilia A.

22 years ago
Do you have an example script that can be used to reproduce the supposed memory access violation? Ilia On June 14, 2004 01:51 pm, Alexander Valyalkin wrote:

Christian Schneider

22 years ago
Ilia Alshanetsky wrote:
> Do you have an example script that can be used to reproduce the supposed > memory access violation?
The function php_basename actually DOES access the char at s-1, i.e. one byte before the string it is passed. If you hand it a buffer gotten from malloc you get a valgrind hit. If I use emalloc this doesn't happen, I guess emalloc has some header stuff before the allocation. If for whatever reason s[-1] contains '/' then things would get even worse. Conclusion: The internal function php_basename IS accessing one byte before the buffer passed to it but the PHP environment seems to hide this behaviour. Is this a bug which should proactively been fixed? Or do we rely on the environment to guarantee that we can access s[-1] without problems? I don't know and it's up to you PHP gods to decide ;-) - Chris

Stefan Esser

22 years ago
Hi,
> The function php_basename actually DOES access the char at s-1, i.e. one > byte before the string it is passed. If you hand it a buffer gotten from > malloc you get a valgrind hit. If I use emalloc this doesn't happen, I > guess emalloc has some header stuff before the allocation. If for > whatever reason s[-1] contains '/' then things would get even worse.
First of all: yes it should be fixed, but it is nothing dangerous. emalloc stores the size of the buffer before the pointer you actually get. On little endian systems this means a '/' can only be directly before the buffer if this buffer is > 752MB big. For big endian systems this this is of course easier ;) Anyway this size is used only by the memory cache within the ZendEngine and by the memory_limit check. So by overwriting with a zero you could make the ZendEngine believe more memory is allocated that really is. <- No problem. The other thing could be that the block is inserted into the wrong cache position. Because we can only write 0 bytes this is no problem, because we would always insert the free memory block in a block that is assumed to be smaller (so no overflow possible). Stefan

Andi Gutmans

22 years ago
I don't think it's a good idea to access s[-1] even if the logic works. This should be fixed IMO. At 10:30 PM 6/14/2004 +0200, Christian Schneider wrote: