This is a diff that should fix a few issues I've encountered with sftp's
tab-complete, and a few others that I found in the process.
1) In makeargs: Lack of bounds checking for the argument pointer
table, causing a buffer overflow when entering too many arguments.
2)Improper handling of absolute paths when PWD is part of the completed
path.
ex:
sftp> cd /usr/local
sftp> ls /usr/loc<tab>
expected result: /usr/local/
actual result: /usr/loc
3)Improper handling of filenames containing escaped globbing characters.
ex:
touch "file*name"
sftp> ls file\*<tab>
expected result: file\*name
actual result: file\*ame
4) * and # aren't getting escaped.
ex:
sftp> ls file<tab>
expected result: file\*name
actual result: file*name
The character # causes makeargs to ignore the rest of the argument.
So if you type "rm important_file#2" you will end up with the
following arguments:
[0] "rm"
[1] "important_file"
ex:
sftp> rm important_file#2
Removing /tmp/important_file
I haven't seen anywhere where this is documented. I added it to the
characters to escape but I don't know why this behaviour is there to
begin with.
Index: sftp.c
===================================================================
RCS file: /cvs/src/usr.bin/ssh/sftp.c,v
retrieving revision 1.136
diff -u -r1.136 sftp.c
--- sftp.c 22 Jun 2012 14:36:33 -0000 1.136
+++ sftp.c 11 Sep 2012 00:48:37 -0000
@@ -968,6 +968,10 @@
state = MA_START;
i = j = 0;
for (;;) {
+ if (argc >= sizeof(argv) / sizeof(*argv)){
+ error("Too many arguments.");
+ return NULL;
+ }
if (isspace(arg[i])) {
if (state == MA_UNQUOTED) {
/* Terminate current argument */
@@ -1671,7 +1675,7 @@
{
glob_t g;
char *tmp, *tmp2, ins[3];
- u_int i, hadglob, pwdlen, len, tmplen, filelen;
+ u_int i, hadglob, pwdlen, len, tmplen, filelen, cesc, isesc, isabs;
const LineInfo *lf;
/* Glob from "file" location */
@@ -1680,6 +1684,9 @@
else
xasprintf(&tmp, "%s*", file);
+ /* Check if the path is absolute. */
+ isabs = tmp[0] == '/';
+
memset(&g, 0, sizeof(g));
if (remote != LOCAL) {
tmp = make_absolute(tmp, remote_path);
@@ -1714,7 +1721,7 @@
goto out;
tmp2 = complete_ambiguous(file, g.gl_pathv, g.gl_matchc);
- tmp = path_strip(tmp2, remote_path);
+ tmp = path_strip(tmp2, isabs ? NULL : remote_path);
xfree(tmp2);
if (tmp == NULL)
@@ -1723,8 +1730,24 @@
tmplen = strlen(tmp);
filelen = strlen(file);
- if (tmplen > filelen) {
- tmp2 = tmp + filelen;
+ /*
+ *Count the number of escaped characters in the
+ *input string.
+ */
+ cesc = isesc = 0;
+ for(i = 0; i < filelen; i++)
+ {
+ if (!isesc && file[i] == '\\' && i + 1 < filelen){
+ isesc = 1;
+ cesc++;
+ }
+ else
+ isesc = 0;
+ }
+
+
+ if (tmplen > (filelen - cesc)) {
+ tmp2 = tmp + filelen - cesc;
len = strlen(tmp2);
/* quote argument on way out */
for (i = 0; i < len; i++) {
@@ -1738,6 +1761,8 @@
case '\t':
case '[':
case ' ':
+ case '#':
+ case '*':
if (quote == '\0' || tmp2[i] == quote) {
if (el_insertstr(el, ins) == -1)
fatal("el_insertstr "
