SFTPException: Cannot handle values > Long.MAX_VALUE Reading Directory Attributes if Server Returns -1 as Directory Size

[jeremykwiatkowski]

I'm getting the following exception when the sFTP server decides to return a (bogus) size for a directory (in this specific case, -1):

Caused by: net.schmizz.sshj.sftp. SFTPException: Cannot handle values > Long.MAX_VALUE
               at net.schmizz.sshj.sftp.SFTPPacket.readFileAttributes(SFTPPacket.java:55)
               at net.schmizz.sshj.sftp.SFTPEngine.stat(SFTPEngine.java:361)
               at net.schmizz.sshj.sftp.SFTPEngine.stat(SFTPEngine.java:244)
               at net.schmizz.sshj.sftp.SFTPClient.statExistence(SFTPClient.java:121)
               at net.schmizz.sshj.sftp.SFTPClient.mkdirs(SFTPClient.java:104)
               ... 6 more
Caused by: net.schmizz.sshj.common.Buffer$BufferException: Cannot handle values > Long.MAX_VALUE
               at net.schmizz.sshj.common.Buffer.readUInt64(Buffer.java:366)
               at net.schmizz.sshj.sftp.SFTPPacket.readFileAttributes(SFTPPacket.java:42)

In my particular case, I traced this to a change in the Apache org.apache.sshd:sshd-sftp code, specifically in the org.apache.sshd.sftp.common.SftpHelper.writeAttrsV3 method. For a directory:

In 2.1.15 writes (returns) 0c -> PERMISSIONS & ACMODTIME
In 2.1.16 writes (returns) 0d -> SIZE & PERMISSIONS & ACMODTIME

Where the size value is all ones (-1 if it were interpreted as signed).

    public static final int SSH_FILEXFER_ATTR_SIZE = 0x00000001;
    public static final int SSH_FILEXFER_ATTR_UIDGID = 0x00000002;
    public static final int SSH_FILEXFER_ATTR_PERMISSIONS = 0x00000004;
    public static final int SSH_FILEXFER_ATTR_ACMODTIME = 0x00000008; // v3 naming convention
    public static final int SSH_FILEXFER_ATTR_ACCESSTIME = 0x00000008; // v4

The "all ones" value for the size then causes the exception raised (intentionally) by the readUInt64() call in net.schmizz.sshj.sftp.SFTPPacket.readFileAttributes():

   public FileAttributes readFileAttributes()
            throws SFTPException {
        final FileAttributes.Builder builder = new FileAttributes.Builder();
        try {
            final int mask = readUInt32AsInt();
            if (FileAttributes.Flag.SIZE.isSet(mask))
                builder.withSize(readUInt64());
            if (FileAttributes.Flag.UIDGID.isSet(mask))
                builder.withUIDGID(readUInt32AsInt(), readUInt32AsInt());

The spec (https://datatracker.ietf.org/doc/html/draft-ietf-secsh-filexfer-02#page-8) doesn't seem to provide any clear indication about what valid values are for the length attribute when information about a directory is being returned. (This field is defined as uint64, i.e., unsigned, so when I say "-1" here I'm referring to the Java long (signed) interpretation of these bits.)

From the Apache server side, I found this issue where the change was made: apache/mina-sshd#751 with the following explanation (from the commit message):

However, there were a few problems.

First, while the "long name" would contain a size also for directory
entries, the corresponding SFTP attributes always had size zero. Fix
this by reporting the size in the attributes not only for files but
simply for all entries, including directories.

Perhaps this is all accidental, in the sense that there was never any specific decision to return -1 for a directory.

Nevertheless, lacking a clear prohibition on that value, it seems like sshj would be more robust if it did not raise that exception, since it does not appear to be prohibited by the spec.

I suppose the options would be:

1. Don't raise the exception for 64 bit unsigned length values that don't fit into a long. This seems like the simplest. From a Java perspective, the caller would get a length of -1 in this case, but the "all ones" value is what the server returned. It is relatively straightforward, though annoying, to convert an unsigned 64bit value in a long into a BigInteger.

2. Avoid the negative value if, for example, we know its a directory (not familiar enough with the code to know if that is possible).

3. Use BigInteger (which has API compatibility issues)

Finally, I'd note that this issue is somewhat similar to the one addressed by #513 (in the sense that its an unsigned/signed 64 bit issue), though the solution in that case was to use BigInteger.

Thanks to anyone who read this far ;-)

Jeremy