Bug 861 - Swapped parameters of SSH_FXP_SYMLINK packet of SFTP protocol
Summary: Swapped parameters of SSH_FXP_SYMLINK packet of SFTP protocol
Status: CLOSED WONTFIX
Alias: None
Product: Portable OpenSSH
Classification: Unclassified
Component: sftp-server (show other bugs)
Version: -current
Hardware: All All
: P2 normal
Assignee: OpenSSH Bugzilla mailing list
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2004-05-04 17:35 AEST by Martin Prikryl
Modified: 2006-10-07 11:36 AEST (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Prikryl 2004-05-04 17:35:02 AEST
Hello,

I'm developer of WinSCP (SCP/SFTP client for Windows).
I have just realised that my client is probably sending parameters linkpath and 
targetpath of SFTP SSH_FXP_SYMLINK packet in incorrect order. What has 
surprised me is that despite this, it works fine with all OpenSSH SFTP servers. 
Well otherwise I would have noticed earlier :-) This made me believe that 
OpenSSH SFTP server has the same bug.
Ssh.com SFTP server obviously adheres to the standards, so with it my client 
cannot create the links.

Can you confirm this?

http://www.openssh.org/txt/draft-ietf-secsh-filexfer-02.txt

The SSH_FXP_SYMLINK request will create a symbolic link on the
server.  It is of the following format

   	uint32     id
   	string     linkpath
   	string     targetpath

The latest (CVS) sftp-server.c:

873:    oldpath = get_string(NULL);
874:    newpath = get_string(NULL);
875:    TRACE("symlink id %u old %s new %s", id, oldpath, newpath);
876:    /* this will fail if 'newpath' exists */
877:    ret = symlink(oldpath, newpath);

The same bug is obviously even in OpenSSH SFTP client:
The latest (CVS) sftp-client.c:

647:    id = conn->msg_id++;
648:    buffer_put_char(&msg, SSH2_FXP_SYMLINK);
649:    buffer_put_int(&msg, id);
650:    buffer_put_cstring(&msg, oldpath);
651:    buffer_put_cstring(&msg, newpath);

Have a nice day.

Martin Prikryl
http://www.prikryl.cz/
http://winscp.sourceforge.net/
Comment 1 Darren Tucker 2004-05-12 12:40:33 AEST
Yep, that looks like a bug to me.  I suspect the confusion is due to
SSH_FXP_SYMLINK having its arguments in the opposite order to the equivalent
Unix "ln" command, whereas other (eg FXP_RENAME) are the same as their equivalents.

The next question is what to do about it, and when?  AFAIK, sftp doesn't know
what SSH server is on the other end, except for the SFTP protocol version (it
just runs "ssh -s host sftp").  Maybe fix it next time the protocol version
supported is bumped?
Comment 2 Martin Prikryl 2004-05-12 17:52:56 AEST
Are there any plans to support newer version of SFTP? You support version 3, 
while there is version 5 draft published already.

BTW: version 5 has mechanism to detect version of SFTP server software (not 
SFTP protocol version).

Possible (maybe dirty) workaround would be to utilize SFTP extension to let the 
client know if the bug is fixed already. But it would not solve problem when 
other vendors's client connects to your server.
Comment 3 Damien Miller 2005-02-09 15:30:15 AEDT
I have thought about adding filexfer-06 support but every time I go to read the
internet-draft again I am repulsed by all the unnecessary bloat that has crept
in. Not to mention that the filexfer negotiation mechanism requires that we
implement versions 4 and 5 of the protocol too.

We could use an extension to detect this bug, but I recall some discussion on
the working group mailing list that SSH_FXP_INIT extensions were removed because
of incompatibility with filexfer-02 clients. 

Therefore, we would need to use a SSH_FXP_EXTENDED packet. This would need an
additional round-trip and would only work for clients that know about the
extension anyway. I also think that it is overkill to do all this to fix a
argument-swapping bug. So I think this solution is pretty useless :(

To avoid breaking deployed software, I think that the only "solution" is to
leave the bug as it is and fix it if/when we add support for a newer filexfer
version.
Comment 4 Darren Tucker 2006-10-07 11:36:20 AEST
Change all RESOLVED bug to CLOSED with the exception of the ones fixed post-4.4.