Bug 1541 - sftp: the do_stat() failure is ignored for chown, chgrp ops. in parse_dispatch_command()
Summary: sftp: the do_stat() failure is ignored for chown, chgrp ops. in parse_dispatc...
Status: CLOSED FIXED
Alias: None
Product: Portable OpenSSH
Classification: Unclassified
Component: sftp (show other bugs)
Version: -current
Hardware: All All
: P5 trivial
Assignee: Assigned to nobody
URL:
Keywords:
Depends on:
Blocks: V_5_2
  Show dependency treegraph
 
Reported: 2008-11-25 00:21 AEDT by SashaN
Modified: 2009-02-23 13:36 AEDT (History)
1 user (show)

See Also:


Attachments
propagate error on failure inside chown/chgrp (919 bytes, patch)
2008-12-08 09:24 AEDT, Damien Miller
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description SashaN 2008-11-25 00:21:48 AEDT
This is a realy small bug.

the code in parse_dispatch_command() looks as follows
@1186
        for (i = 0; g.gl_pathv[i] && !interrupted; i++) {
            if (!(aa = do_stat(conn, g.gl_pathv[i], 0))) {
                if (err != 0 && err_abort)
                    break;
                else
                    continue;
the err is always 0, once for loop is entered (this is guaranteed by code in parse_dispatch_command()
so if do_stat() operation fails, the if (err != 0 && err_abort) is still not satisfied and continue is always
executed. this prevents sftp client to bail out the function on the first failure.

this will cause problem only in batchmode.


--- cut here to get patch ------
bash-3.2$ diff -u /net/grok.czech/ws-local/onnv-clone/usr/src/cmd/ssh/sftp/sftp.c sftp.c
--- /net/grok.czech/ws-local/onnv-clone/usr/src/cmd/ssh/sftp/sftp.c     Wed Oct 15 00:41:07 2008
+++ sftp.c      Fri Nov 21 17:05:48 2008
@@ -1185,18 +1185,26 @@
                remote_glob(conn, path1, GLOB_NOCHECK, NULL, &g);
                for (i = 0; g.gl_pathv[i] && !interrupted; i++) {
                        if (!(aa = do_stat(conn, g.gl_pathv[i], 0))) {
-                               if (err != 0 && err_abort)
+                               if (err_abort) {
+                                       err = -1;
                                        break;
-                               else
+                               }
+                               else {
+                                       err = 0;
                                        continue;
+                               }
                        }
                        if (!(aa->flags & SSH2_FILEXFER_ATTR_UIDGID)) {
                                error("Can't get current ownership of "
                                    "remote file \"%s\"", g.gl_pathv[i]);
-                               if (err != 0 && err_abort)
+                               if (err_abort) {
+                                       err = -1;
                                        break;
-                               else
+                               }
+                               else {
+                                       err = 0;
                                        continue;
+                               }
                        }
                        aa->flags &= SSH2_FILEXFER_ATTR_UIDGID;
                        if (cmdnum == I_CHOWN) {
Comment 1 Damien Miller 2008-12-08 09:24:52 AEDT
Created attachment 1584 [details]
propagate error on failure inside chown/chgrp
Comment 2 Damien Miller 2008-12-08 09:25:17 AEDT
Put this on the list for openssh-5.2
Comment 3 Damien Miller 2008-12-09 14:13:19 AEDT
Fix applied. This will be in openssh-5.2 - thanks!
Comment 4 Damien Miller 2009-02-23 13:36:42 AEDT
Close bugs fixed/reviewed for openssh-5.2 release