Hello,
I think I found a use-after-free bug and a manpage discrepancy.
1. ftp>newer prints nothing or garbage instead of the local filename,
when alerting that the local file is newer than the remote file.
I think this is because getit(argc, argv, -1, "w") frees globargv2,
which is set equal to argv2, making fprintf(... argv[2]) that follows
into a use-after-free.
2. The manpage for ftp(1) says that ftp>mget -n uses ftp>newer instead
of ftp>get. This is false, ftp>mget -n calls getit(argc,argv,-1,"w")
directly. ftp>mget -n never triggers the alert message that ftp>newer
does when the local file is newer than the remote file.
I tried fixing both issues below. The first diff aims to fix both the
bug and the discrepancy by removing the alert from the definition of
newer(). The second diff tries to restore the alert by placing it
appropriately inside getit(). Sorry for the broken line, I was keeping
the (awful) indentation style of the original file. Also apologies if
I've misunderstood the code: I haven't done serious programming before.
--Vladimir Sotirov
P.S. (I was thinking of implementing three actual changes to ftp(1),
but since I lack experience I'd prefer to do so only after a go-ahead
that the ideas are not stupid.
1. Make ftp>mget -n and ftp>mget -c exclusive switches, right now
whichever's last wins (e.g. ftp>mget -nc acts like ftp>mget -c but
ftp>mget -cn acts like ftp>mget -n).
2. Make it so ftp>reget presumes a local file is a partially complete
transfer only if the local file is newer than the remote file. That
might prevent wasting bandwidth in the unfortunate situation of
resuming a transfer inbetween the remote files having changed. It will
also make ftp>reget semantically exclusive to ftp>newer.
3. Add an ftp -N switch that acts like the autofetch mode of ftp -C but
uses ftp>newer instead of ftp>reget.)
Index: cmds.c
===================================================================
RCS file: /cvs/src/usr.bin/ftp/cmds.c,v
retrieving revision 1.76
diff -u -p -r1.76 cmds.c
--- cmds.c 17 Mar 2016 19:40:43 -0000 1.76
+++ cmds.c 25 May 2016 04:40:27 -0000
@@ -1643,9 +1643,7 @@ void
newer(int argc, char *argv[])
{
- if (getit(argc, argv, -1, "w"))
- fprintf(ttyout, "Local file \"%s\" is newer than remote file
\"%s\".\n",
- argv[2], argv[1]);
+ (void)getit(argc, argv, -1, "w");
}
/*
Index: small.c
===================================================================
RCS file: /cvs/src/usr.bin/ftp/small.c,v
retrieving revision 1.5
diff -u -p -r1.5 small.c
--- small.c 18 Oct 2015 03:04:11 -0000 1.5
+++ small.c 25 May 2016 04:40:27 -0000
@@ -275,6 +275,10 @@ usage:
goto freegetit;
if (stbuf.st_mtime >= mtime) {
rval = 1;
+ fprintf(ttyout,
+ "Local file \"%s\" is newer "\
+ "than remote file \"%s\".\n",
+ argv[2], argv[1]);
goto freegetit;
}
}
