# bug #39863 in trunk/5.4 - Thread: https://externals.io/message/52913 - Original archive: https://news-web.php.net/php.internals/52913 - Started by Stas Malyshev on 2011-06-05T08:25:09+00:00 - Messages: 9 - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #52913 — Stas Malyshev — 2011-06-05T08:25:09+00:00 URL: https://externals.io/message/52913#52913 Hi! As far as I can see, bug 39863 was fixed in 5.3, but the fix still not in trunk/5.4. Should we merge the same patch into trunk/5.4 or somebody is volunteering to fix it, e.g. like described here: http://news.php.net/php.internals/50191? See also the about it discussion: http://www.serverphorums.com/read.php?7,230402 -- Stanislav Malyshev, Software Architect SugarCRM: http://www.sugarcrm.com/ (408)454-6900 ext. 227 --- ## #52914 — Pierre Joye — 2011-06-05T10:15:43+00:00 URL: https://externals.io/message/52913#52914 · In reply to: #52913 hi, Not apparently, it was not fixed in trunk. There was a discussion about using zend_arg for paths and additional function or macros to be used instead of duplicating the tests everywhere. But no consensus or agreement have been reached. Cheers, > Hi! > > As far as I can see, bug 39863 was fixed in 5.3, but the fix still not in > trunk/5.4. > Should we merge the same patch into trunk/5.4 or somebody is volunteering to > fix it, e.g. like described here: http://news.php.net/php.internals/50191? > See also the about it discussion: > http://www.serverphorums.com/read.php?7,230402 > -- > Stanislav Malyshev, Software Architect > SugarCRM: http://www.sugarcrm.com/ > (408)454-6900 ext. 227 > > -- > > > > -- Pierre @pierrejoye | http://blog.thepimp.net | http://www.libgd.org --- ## #52930 — Felipe Pena — 2011-06-05T12:47:26+00:00 URL: https://externals.io/message/52913#52930 · In reply to: #52914 Hi, 2011/6/5 Pierre Joye > hi, > > Not apparently, it was not fixed in trunk. > > There was a discussion about using zend_arg for paths and additional > function or macros to be used instead of duplicating the tests > everywhere. But no consensus or agreement have been reached. > > Should http://felipe.ath.cx/diff/parse_arg_null_path.diff be enough (beyond changing others function parse args, fixing the tests, include+require part)? $ sapi/cli/php -r 'fopen("a\0b", "r");' Warning: `fopen()` expects parameter 1 to be valid path, string given in Command line code on line 1 Thanks. -- Regards, Felipe Pena --- ## #52954 — Stas Malyshev — 2011-06-05T19:09:34+00:00 URL: https://externals.io/message/52913#52954 · In reply to: #52930 Hi! > Should http://felipe.ath.cx/diff/parse_arg_null_path.diff be enough > (beyond changing others function parse args, fixing the tests, > include+require part)? > > $ sapi/cli/php -r 'fopen("a\0b", "r");' > > Warning: `fopen()` expects parameter 1 to be valid path, string given in > Command line code on line 1 This should be applied not only to fopen but to any function that does anything with filenames (and include/require/etc. also, I guess). -- Stanislav Malyshev, Software Architect SugarCRM: http://www.sugarcrm.com/ (408)454-6900 ext. 227 --- ## #52955 — Felipe Pena — 2011-06-05T19:13:45+00:00 URL: https://externals.io/message/52913#52955 · In reply to: #52954 2011/6/5 Stas Malyshev > Hi! > > > Should http://felipe.ath.cx/diff/parse_arg_null_path.diff be enough > > […] > > This should be applied not only to fopen but to any function that does > anything with filenames (and include/require/etc. also, I guess). > > Of course, I was just checking if it's what you guys are thinking first. -- Regards, Felipe Pena --- ## #52956 — Pierre Joye — 2011-06-05T19:18:37+00:00 URL: https://externals.io/message/52913#52956 · In reply to: #52955 > 2011/6/5 Stas Malyshev > > […] > > Of course, I was just checking if it's what you guys are thinking first. yes, that's the idea, to add a argument for zend_parse and a function/macro for other areas. Cheers, -- Pierre @pierrejoye | http://blog.thepimp.net | http://www.libgd.org --- ## #52957 — Stas Malyshev — 2011-06-05T19:19:21+00:00 URL: https://externals.io/message/52913#52957 · In reply to: #52955 Hi! > Of course, I was just checking if it's what you guys are thinking first. Well, there was basically two ideas: 1. Add filename length to streams and check inside streams 2. Check inside argument parser Both have downsides: (1) does not capture cases when we don't use streams (such as direct stat/touch/etc functions), (2) doesn't cover the case when stream is manipulated through a string not coming directly from a function argument (e.g. include, but may be other cases with extensions). So, ideally, it'd be nice to have both - or something third that I didn't think of - but any of them is better than nothing. (1) seems to be easier and less disruptive, provided that we cover include case separately and locate all functions that deal with filenames. -- Stanislav Malyshev, Software Architect SugarCRM: http://www.sugarcrm.com/ (408)454-6900 ext. 227 --- ## #53087 — Felipe Pena — 2011-06-06T21:53:11+00:00 URL: https://externals.io/message/52913#53087 · In reply to: #52957 Hi, 2011/6/5 Stas Malyshev > Hi! > > > Of course, I was just checking if it's what you guys are thinking first. > > […] > > Well, there was basically two ideas: > 1. Add filename length to streams and check inside streams > 2. Check inside argument parser > > Both have downsides: (1) does not capture cases when we don't use streams > (such as direct stat/touch/etc functions), (2) doesn't cover the case when > stream is manipulated through a string not coming directly from a function > argument (e.g. include, but may be other cases with extensions). So, > ideally, it'd be nice to have both - or something third that I didn't think > of - but any of them is better than nothing. > (1) seems to be easier and less disruptive, provided that we cover include > case separately and locate all functions that deal with filenames. > > Ok, I've committed in 5.4 and trunk the argument parser part. Now I need to fix some tests and try to found other places needing for related checks. Thanks. -- Regards, Felipe Pena --- ## #53094 — Stas Malyshev — 2011-06-06T22:49:17+00:00 URL: https://externals.io/message/52913#53094 · In reply to: #53087 Hi! > Ok, I've committed in 5.4 and trunk the argument parser part. > > Now I need to fix some tests and try to found other places needing for > related checks. Thanks for fixing it! -- Stanislav Malyshev, Software Architect SugarCRM: http://www.sugarcrm.com/ (408)454-6900 ext. 227