Re: Fix for sftp(1) tab-complete

2012-09-21 Thread Darren Tucker
On Tue, Sep 11, 2012 at 11:27 AM, Jean-Marc Robert
jeanmarc.rob...@gmail.com wrote:
 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.

Thanks, these have been committed (with some mild style adjustment).

-- 
Darren Tucker (dtucker at zip.com.au)
GPG key 8FF4FA69 / D9A3 86E9 7EEE AF4B B2D4  37C9 C982 80C7 8FF4 FA69
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.



Fix for sftp(1) tab-complete

2012-09-10 Thread Jean-Marc Robert
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/loctab

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 filetab
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 -  1.136
+++ sftp.c  11 Sep 2012 00:48:37 -
@@ -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