* segfault with exceedingly long path @ 2014-01-18 0:20 Axel Beckert 2014-01-18 1:49 ` Bart Schaefer 0 siblings, 1 reply; 12+ messages in thread From: Axel Beckert @ 2014-01-18 0:20 UTC (permalink / raw) To: zsh-workers Hi, this is a forward of http://bugs.debian.org/418199 --->8--- Running the following on a tmpfs makes zsh segfault: % for i in `seq 1000`; do mkdir 0123456789; cd 0123456789; done; cd .. ---8<--- I was able to reproduce this segfault with zsh 5.0.5 as currently in Debian Testing/Unstable. Oldest version of zsh known to be affected is 4.3.2. (Currently going through unresolved bugs against zsh in Debian which involve segfaults.) Kind regards, Axel -- /~\ Plain Text Ribbon Campaign | Axel Beckert \ / Say No to HTML in E-Mail and News | abe@deuxchevaux.org (Mail) X See http://www.nonhtmlmail.org/campaign.html | abe@noone.org (Mail+Jabber) / \ I love long mails: http://email.is-not-s.ms/ | http://noone.org/abe/ (Web) ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: segfault with exceedingly long path 2014-01-18 0:20 segfault with exceedingly long path Axel Beckert @ 2014-01-18 1:49 ` Bart Schaefer 2014-01-18 9:11 ` Axel Beckert 2014-01-19 19:10 ` Peter Stephenson 0 siblings, 2 replies; 12+ messages in thread From: Bart Schaefer @ 2014-01-18 1:49 UTC (permalink / raw) To: zsh-workers On Jan 18, 1:20am, Axel Beckert wrote: } } this is a forward of http://bugs.debian.org/418199 This is a known issue and unlikely ever to be fixed. Various parts of the shell rely on system limits such as PATH_MAX which cannot be dynamically changed. There's a comment with some explanation around lines 109-137 of Src/compat.c. The upshot is that if you try hard enough you can always create a path that will exceed some limit or other. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: segfault with exceedingly long path 2014-01-18 1:49 ` Bart Schaefer @ 2014-01-18 9:11 ` Axel Beckert 2014-01-19 19:10 ` Peter Stephenson 1 sibling, 0 replies; 12+ messages in thread From: Axel Beckert @ 2014-01-18 9:11 UTC (permalink / raw) To: zsh-workers Hi Bart, On Fri, Jan 17, 2014 at 05:49:02PM -0800, Bart Schaefer wrote: > On Jan 18, 1:20am, Axel Beckert wrote: > } this is a forward of http://bugs.debian.org/418199 > > This is a known issue and unlikely ever to be fixed. Thanks for the information. Marking as "won't fix". Kind regards, Axel -- /~\ Plain Text Ribbon Campaign | Axel Beckert \ / Say No to HTML in E-Mail and News | abe@deuxchevaux.org (Mail) X See http://www.nonhtmlmail.org/campaign.html | abe@noone.org (Mail+Jabber) / \ I love long mails: http://email.is-not-s.ms/ | http://noone.org/abe/ (Web) ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: segfault with exceedingly long path 2014-01-18 1:49 ` Bart Schaefer 2014-01-18 9:11 ` Axel Beckert @ 2014-01-19 19:10 ` Peter Stephenson 2014-01-19 21:35 ` Bart Schaefer 1 sibling, 1 reply; 12+ messages in thread From: Peter Stephenson @ 2014-01-19 19:10 UTC (permalink / raw) To: zsh-workers On Fri, 17 Jan 2014 17:49:02 -0800 Bart Schaefer <schaefer@brasslantern.com> wrote: > On Jan 18, 1:20am, Axel Beckert wrote: > } > } this is a forward of http://bugs.debian.org/418199 > > This is a known issue and unlikely ever to be fixed. Various parts > of the shell rely on system limits such as PATH_MAX which cannot be > dynamically changed. There's a comment with some explanation around > lines 109-137 of Src/compat.c. > > The upshot is that if you try hard enough you can always create a path > that will exceed some limit or other. I'm still in favour of reducing dependency on PATH_MAX wherever possible, since any arbitrary limit we don't actually need we don't actually want, but with 81 occurences in source and header files this isn't a battle that's going to yield any instant gratification. pws ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: segfault with exceedingly long path 2014-01-19 19:10 ` Peter Stephenson @ 2014-01-19 21:35 ` Bart Schaefer 2014-01-19 22:13 ` Simon Ruderich 0 siblings, 1 reply; 12+ messages in thread From: Bart Schaefer @ 2014-01-19 21:35 UTC (permalink / raw) To: zsh-workers On Jan 19, 7:10pm, Peter Stephenson wrote: } } I'm still in favour of reducing dependency on PATH_MAX wherever } possible, since any arbitrary limit we don't actually need we don't } actually want In this particular case even if the shell were able to deal with an "unlimited" directory nesting depth, system calls like chdir(2) would begin to fail. fchdir() sort of works around it by not passing the path name, but you still must somehow obtain a file descriptor to the directory, so it's just pushing the problem up a level. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: segfault with exceedingly long path 2014-01-19 21:35 ` Bart Schaefer @ 2014-01-19 22:13 ` Simon Ruderich 2014-01-20 0:02 ` Bart Schaefer 0 siblings, 1 reply; 12+ messages in thread From: Simon Ruderich @ 2014-01-19 22:13 UTC (permalink / raw) To: zsh-workers [-- Attachment #1: Type: text/plain, Size: 908 bytes --] On Sun, Jan 19, 2014 at 01:35:50PM -0800, Bart Schaefer wrote: > In this particular case even if the shell were able to deal with an > "unlimited" directory nesting depth, system calls like chdir(2) would > begin to fail. PATH_MAX is the maximum relative path length, which should be accepted by chdir(2) and similar system calls. As long as we don't use paths longer than PATH_MAX in system calls, it doesn't matter how "deep" the directory structure is. It only becomes a problem if a very long absolute path is used. So we could do that in small steps, e.g. first chdir(2) to a directory < PATH_MAX bytes deep, then from there to the next directory < PATH_MAX bytes deep and so on until we get down to the real directory. I don't think that's worth it, but it works. Regards Simon -- + privacy is necessary + using gnupg http://gnupg.org + public key id: 0x92FEFDB7E44C32F9 [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: segfault with exceedingly long path 2014-01-19 22:13 ` Simon Ruderich @ 2014-01-20 0:02 ` Bart Schaefer 2014-01-20 1:10 ` Bart Schaefer 0 siblings, 1 reply; 12+ messages in thread From: Bart Schaefer @ 2014-01-20 0:02 UTC (permalink / raw) To: zsh-workers On Jan 19, 11:13pm, Simon Ruderich wrote: } } So we could do that in small steps, e.g. first chdir(2) to a } directory < PATH_MAX bytes deep, then from there to the next } directory < PATH_MAX bytes deep and so on until we get down to } the real directory. Yes, but that doesn't even begin to address all the other places where very long paths become an issue. Would redirections have to implcitly chdir along the pathname until they got "close enough" to pass the path tail to open()? Can you put a ridiculously long path into $path or $fpath or $module_path? $TMPPREFIX? $ZDOTDIR? What does [[ -d $longpath ]] do? zstat $longpath? ${(A)longpath}? Can you even do an execve() when $#PWD is longer than PATH_MAX? I don't think we want to let this can of worms out of Pandora's box, or we'll be chasing geese until the cows come home to roost. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: segfault with exceedingly long path 2014-01-20 0:02 ` Bart Schaefer @ 2014-01-20 1:10 ` Bart Schaefer 2014-01-20 1:44 ` Bart Schaefer ` (2 more replies) 0 siblings, 3 replies; 12+ messages in thread From: Bart Schaefer @ 2014-01-20 1:10 UTC (permalink / raw) To: zsh-workers On Jan 19, 4:02pm, Bart Schaefer wrote: } } I don't think we want to let this can of worms out of Pandora's box, } or we'll be chasing geese until the cows come home to roost. In spite of that ... we could at least not dump core in this specific case. There are probably many other core dumps waiting to be exposed. Behavior becomes undefined once the path gets too long, but: diff --git a/Src/utils.c b/Src/utils.c index c6d178c..705d2c4 100644 --- a/Src/utils.c +++ b/Src/utils.c @@ -725,32 +725,36 @@ xsymlinks(char *s) char **pp, **opp; char xbuf2[PATH_MAX*2], xbuf3[PATH_MAX*2]; int t0, ret = 0; + zulong xbuflen = strlen(xbuf); opp = pp = slashsplit(s); - for (; *pp; pp++) { - if (!strcmp(*pp, ".")) { - zsfree(*pp); + for (; xbuflen < sizeof(xbuf) && *pp; pp++) { + if (!strcmp(*pp, ".")) continue; - } if (!strcmp(*pp, "..")) { char *p; - zsfree(*pp); if (!strcmp(xbuf, "/")) continue; if (!*xbuf) continue; - p = xbuf + strlen(xbuf); - while (*--p != '/'); + p = xbuf + xbuflen; + while (*--p != '/') + xbuflen--; *p = '\0'; continue; } sprintf(xbuf2, "%s/%s", xbuf, *pp); t0 = readlink(unmeta(xbuf2), xbuf3, PATH_MAX); if (t0 == -1) { - strcat(xbuf, "/"); - strcat(xbuf, *pp); - zsfree(*pp); + zulong pplen = strlen(pp) + 1; + if ((xbuflen += pplen) < sizeof(xbuf)) { + strcat(xbuf, "/"); + strcat(xbuf, *pp); + } else { + *xbuf = 0; + break; + } } else { ret = 1; metafy(xbuf3, t0, META_NOALLOC); @@ -759,10 +763,9 @@ xsymlinks(char *s) xsymlinks(xbuf3 + 1); } else xsymlinks(xbuf3); - zsfree(*pp); } } - free(opp); + freearray(opp); return ret; } @@ -779,8 +782,10 @@ xsymlink(char *s) return NULL; *xbuf = '\0'; xsymlinks(s + 1); - if (!*xbuf) + if (!*xbuf) { + zwarn("path expansion failed, using root directory"); return ztrdup("/"); + } return ztrdup(xbuf); } -- Barton E. Schaefer ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: segfault with exceedingly long path 2014-01-20 1:10 ` Bart Schaefer @ 2014-01-20 1:44 ` Bart Schaefer 2014-01-20 3:20 ` Bart Schaefer 2014-02-24 14:38 ` Oliver Kiddle 2 siblings, 0 replies; 12+ messages in thread From: Bart Schaefer @ 2014-01-20 1:44 UTC (permalink / raw) To: zsh-workers On Jan 19, 5:10pm, Bart Schaefer wrote: } } In spite of that ... we could at least not dump core in this specific } case. Given that this is a buffer overflow, this is probably a security concern as well, because a carefully crafted file path could cause memory to be overwritten with a desired pattern, and a symbolic link could be used to divert the shell into that directory. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: segfault with exceedingly long path 2014-01-20 1:10 ` Bart Schaefer 2014-01-20 1:44 ` Bart Schaefer @ 2014-01-20 3:20 ` Bart Schaefer 2014-02-24 14:38 ` Oliver Kiddle 2 siblings, 0 replies; 12+ messages in thread From: Bart Schaefer @ 2014-01-20 3:20 UTC (permalink / raw) To: zsh-workers On Jan 19, 5:10pm, Bart Schaefer wrote: } } + zulong pplen = strlen(pp) + 1; That should be strlen(*pp). ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: segfault with exceedingly long path 2014-01-20 1:10 ` Bart Schaefer 2014-01-20 1:44 ` Bart Schaefer 2014-01-20 3:20 ` Bart Schaefer @ 2014-02-24 14:38 ` Oliver Kiddle 2014-02-24 15:38 ` Bart Schaefer 2 siblings, 1 reply; 12+ messages in thread From: Oliver Kiddle @ 2014-02-24 14:38 UTC (permalink / raw) To: zsh-workers On 19 Jan, Bart wrote: > - if (!*xbuf) > + if (!*xbuf) { > + zwarn("path expansion failed, using root directory"); > return ztrdup("/"); As a result of this, I now get: zsh -fw cd / zsh: path expansion failed, using root directory So if the directory starts as / the !*xbuf test succeeds and it prints the warning. I'm not sure whether it would be better to skip the whole xsymlinks call for a path of "/" or to check the return status from it. Oliver ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: segfault with exceedingly long path 2014-02-24 14:38 ` Oliver Kiddle @ 2014-02-24 15:38 ` Bart Schaefer 0 siblings, 0 replies; 12+ messages in thread From: Bart Schaefer @ 2014-02-24 15:38 UTC (permalink / raw) To: zsh-workers On Feb 24, 3:38pm, Oliver Kiddle wrote: } } So if the directory starts as / the !*xbuf test succeeds and it prints } the warning. I'm not sure whether it would be better to skip the whole } xsymlinks call for a path of "/" or to check the return status from it. Hrm. xsymlinks() is meant to return 1 when there was a symbolic link followed, but it is also responsible for collapsing out "../" so in xsymlink() the return value doesn't tell us whether the input and output paths should be the same. Also: torch% cd /tmp/../ zsh: path expansion failed, using root directory torch% ln -s / /tmp/root torch% cd /tmp/root zsh: path expansion failed, using root directory So neither checking the return value nor skipping xsymlinks() for "/" will work, we actually have to differentiate paths that resolve to the root from paths that fail to resolve. This is going to require more analysis of xsymlinks() than I have time to do today (possibly more than I have time to do this week). ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2014-02-24 15:38 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-01-18 0:20 segfault with exceedingly long path Axel Beckert 2014-01-18 1:49 ` Bart Schaefer 2014-01-18 9:11 ` Axel Beckert 2014-01-19 19:10 ` Peter Stephenson 2014-01-19 21:35 ` Bart Schaefer 2014-01-19 22:13 ` Simon Ruderich 2014-01-20 0:02 ` Bart Schaefer 2014-01-20 1:10 ` Bart Schaefer 2014-01-20 1:44 ` Bart Schaefer 2014-01-20 3:20 ` Bart Schaefer 2014-02-24 14:38 ` Oliver Kiddle 2014-02-24 15:38 ` Bart Schaefer
Code repositories for project(s) associated with this public inbox https://git.vuxu.org/mirror/zsh/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).