On Tue, 13 Sep 2016 09:29:27 +0200 Tim Ruehsen <[email protected]> wrote:
> On Tuesday, September 13, 2016 5:13:10 AM CEST Matthew White wrote: > > On Mon, 12 Sep 2016 21:20:54 +0200 > > > > Tim Rühsen <[email protected]> wrote: > > > On Montag, 12. September 2016 20:18:30 CEST Eli Zaretskii wrote: > > > > > From: Tim Ruehsen <[email protected]> > > > > > Date: Mon, 12 Sep 2016 13:00:32 +0200 > > > > > > > > > > > + char *basename = name; > > > > > > + > > > > > > + while ((name = strstr (basename, "/"))) > > > > > > + basename = name + 1; > > > > > > > > > > Could you use strrchr() ? something like > > > > > > > > > > char *basename = strrchr (name, '/'); > > > > > > > > > > if (basename) > > > > > > > > > > basename += 1; > > > > > > > > > > else > > > > > > > > > > basename = name; > > > > > > > > I think we want to use ISSEP, no? Otherwise Windows file names with > > > > backslashes will misfire. > > > > > > Good point. What about device names ? > > > > > > So maybe base_name() from Gnulib module 'dirname' is the right choice !? > > > See https://www.gnu.org/software/gnulib/manual/html_node/basename.html > > > > What if Gnulib's base_name() returns "./<basename>"? > > > > libmetalink's metalink_check_safe_path() rejects relative paths: > > https://tools.ietf.org/html/rfc5854#section-4.1.2.1 > > > > Also, basename is used to point to an existing memory location, base_name() > > instead allocates new space. This is not a biggy, but we should keep it in > > mind to amend properly. > > > > lib/basename.c (base_name) > > -------------------------- > > /* On systems with drive letters, "a/b:c" must return "./b:c" rather > > than "b:c" to avoid confusion with a drive letter. On systems > > with pure POSIX semantics, this is not an issue. */ > > -------------------------- > > > > Suggestions? > > ISSEP is "homebrewed" and incomplete but doesn't need a memory allocation. > base_name() is "complete" (the macros check more than just WINDOWS) and we > automatically get improvements from upstream - but it calls malloc(). > > There is also last_component() which returns a pointer to the basename within > your filename. This is basically what you do. > > Anyways, this last component (basename) may still hold a device prefix - you > have to check that with either HAS_DEVICE() (only defined in certain > environments, needs to guarded by #ifdef) or by FILE_SYSTEM_PREFIX_LEN() > which > gives 2 if your basename has a leading device prefix. And you should do this > check in a loop to catch names like 'C:D:xxx'. If you don't do, we likely get > a CVE assigned ;-) Thanks Tim! Yours is a super smart solution! I rewrote src/metalink.c (get_metalink_basename) to skip prefix drive letters on the basename (the declaration of last_component() is in src/metalink.h): #include "dosname.h" char * get_metalink_basename (char *name) { char *basename; if (!name) return NULL; basename = last_component (name); while (FILE_SYSTEM_PREFIX_LEN (basename)) basename += 2; return metalink_check_safe_path (basename) ? basename : NULL; } [make syntax-check is ok, make check-valgrind is ok, contrib/check-hard is ok] WDYT? > > Regards, Tim Regards, Matthew -- Matthew White <[email protected]>
pgpB0PYP4SWIt.pgp
Description: PGP signature
