This is an automated email from the ASF dual-hosted git repository. tomaswolf pushed a commit to branch rc-2.20.0 in repository https://gitbox.apache.org/repos/asf/mina-sshd.git
commit f4f562c29e22ed98479405e8bec58611f2faceba Author: Thomas Wolf <[email protected]> AuthorDate: Sat Sep 19 22:50:06 2026 +0200 Fix check-file-name/check-file-handle SFTP v6 extension Using a small blockSize on a huge file may generate too many hashes. Accumulating them all in a buffer may exhaust the memory. Since most SFTP implementations have a limit on the length of SFTP messages, abort if the buffer gets longer than 256kB. Additionally change the block logic. For a large block size, the previous implementation would allocate a large buffer. Try this with a 1TB file and a 4GB block size... Change this to keep the block size a logical size, and use a fixed-size small buffer to accumulate the digest over large blocks. --- .../sshd/sftp/client/impl/DefaultSftpClient.java | 2 +- .../org/apache/sshd/sftp/common/SftpConstants.java | 3 + .../sftp/server/AbstractSftpSubsystemHelper.java | 113 ++++++++++----------- 3 files changed, 60 insertions(+), 58 deletions(-) diff --git a/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/impl/DefaultSftpClient.java b/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/impl/DefaultSftpClient.java index 422dc5b81..5bc41b284 100644 --- a/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/impl/DefaultSftpClient.java +++ b/sshd-sftp/src/main/java/org/apache/sshd/sftp/client/impl/DefaultSftpClient.java @@ -230,7 +230,7 @@ public class DefaultSftpClient extends AbstractSftpClient { if (length < 5) { throw new IOException("Illegal sftp packet length: " + length); } - if (length > (8 * SshConstants.SSH_REQUIRED_PAYLOAD_PACKET_LENGTH_SUPPORT)) { + if (length > SftpConstants.MAX_SFTP_MESSAGE_LENGTH) { throw new StreamCorruptedException("Illogical sftp packet length: " + length); } if ((wpos - rpos) >= (length + 4)) { diff --git a/sshd-sftp/src/main/java/org/apache/sshd/sftp/common/SftpConstants.java b/sshd-sftp/src/main/java/org/apache/sshd/sftp/common/SftpConstants.java index b95a87736..cc0e5e079 100644 --- a/sshd-sftp/src/main/java/org/apache/sshd/sftp/common/SftpConstants.java +++ b/sshd-sftp/src/main/java/org/apache/sshd/sftp/common/SftpConstants.java @@ -21,6 +21,7 @@ package org.apache.sshd.sftp.common; import java.util.Collections; import java.util.Map; +import org.apache.sshd.common.SshConstants; import org.apache.sshd.common.util.GenericUtils; import org.apache.sshd.common.util.logging.LoggingUtils; @@ -274,6 +275,8 @@ public final class SftpConstants { public static final int SSH_ACL_CAP_INHERIT_ACCESS = 0x00000010; public static final int SSH_ACL_CAP_INHERIT_AUDIT_ALARM = 0x00000020; + public static final int MAX_SFTP_MESSAGE_LENGTH = 8 * SshConstants.SSH_REQUIRED_PAYLOAD_PACKET_LENGTH_SUPPORT; // 256kB + private SftpConstants() { throw new UnsupportedOperationException("No instance"); } diff --git a/sshd-sftp/src/main/java/org/apache/sshd/sftp/server/AbstractSftpSubsystemHelper.java b/sshd-sftp/src/main/java/org/apache/sshd/sftp/server/AbstractSftpSubsystemHelper.java index a2e66fcf3..347fe8021 100755 --- a/sshd-sftp/src/main/java/org/apache/sshd/sftp/server/AbstractSftpSubsystemHelper.java +++ b/sshd-sftp/src/main/java/org/apache/sshd/sftp/server/AbstractSftpSubsystemHelper.java @@ -911,14 +911,14 @@ public abstract class AbstractSftpSubsystemHelper String[] algos = GenericUtils.split(algList, ','); long startOffset = buffer.getLong(); long length = buffer.getLong(); - int blockSize = buffer.getInt(); + long blockSize = buffer.getUInt(); try { buffer = prepareReply(buffer); buffer.putByte((byte) SftpConstants.SSH_FXP_EXTENDED_REPLY); buffer.putInt(id); buffer.putString(SftpConstants.EXT_CHECK_FILE); doCheckFileHash(id, targetType, target, - Arrays.asList(algos), startOffset, length, blockSize, buffer); + Arrays.asList(algos), startOffset, length, (int) blockSize, buffer); } catch (Exception e) { sendStatus(prepareReply(buffer), id, e, SftpConstants.SSH_FXP_EXTENDED, targetType, target, @@ -929,14 +929,18 @@ public abstract class AbstractSftpSubsystemHelper send(buffer); } + protected abstract void doCheckFileHash( + int id, String targetType, String target, Collection<String> algos, + long startOffset, long length, int blockSize, Buffer buffer) throws Exception; + protected void doCheckFileHash( int id, Path file, NamedFactory<? extends Digest> factory, - long startOffset, long length, int blockSize, Buffer buffer) + long startOffset, long length, int unsignedBlockSize, Buffer buffer) throws Exception { ValidateUtils.checkTrue(startOffset >= 0L, "Invalid start offset: %d", startOffset); ValidateUtils.checkTrue(length >= 0L, "Invalid length: %d", length); - ValidateUtils.checkTrue( - (blockSize == 0) || (blockSize >= SftpConstants.MIN_CHKFILE_BLOCKSIZE), + long blockSize = unsignedBlockSize & 0xFFFF_FFFFL; + ValidateUtils.checkTrue(blockSize == 0 || blockSize >= SftpConstants.MIN_CHKFILE_BLOCKSIZE, "Invalid block size: %d", blockSize); Objects.requireNonNull(factory, "No digest factory provided"); buffer.putString(factory.getName()); @@ -954,14 +958,11 @@ public abstract class AbstractSftpSubsystemHelper ValidateUtils.checkTrue(effectiveLength > 0L, "Non-positive effective hash data length: %d", effectiveLength); - byte[] digestBuf = (blockSize == 0) - ? new byte[Math.min((int) effectiveLength, IoUtils.DEFAULT_COPY_SIZE)] - : new byte[Math.min((int) effectiveLength, blockSize)]; + byte[] digestBuf = new byte[IoUtils.DEFAULT_COPY_SIZE]; ByteBuffer wb = ByteBuffer.wrap(digestBuf); SftpFileSystemAccessor accessor = getFileSystemAccessor(); ServerSession session = getServerSession(); - try (SeekableByteChannel channel = accessor.openFile( - this, null, file, null, Collections.emptySet())) { + try (SeekableByteChannel channel = accessor.openFile(this, null, file, null, Collections.emptySet())) { channel.position(startOffset); Digest digest = factory.create(); @@ -969,59 +970,62 @@ public abstract class AbstractSftpSubsystemHelper boolean traceEnabled = log.isTraceEnabled(); if (blockSize == 0) { - while (effectiveLength > 0L) { - int remainLen = Math.min(digestBuf.length, (int) effectiveLength); - ByteBuffer bb = wb; - if (remainLen < digestBuf.length) { - bb = ByteBuffer.wrap(digestBuf, 0, remainLen); - } - bb.clear(); // prepare for next read - - int readLen = channel.read(bb); - if (readLen < 0) { - break; - } + blockSize = effectiveLength; + } - effectiveLength -= readLen; - digest.update(digestBuf, 0, readLen); + long originalLength = effectiveLength; + for (int count = 0; effectiveLength > 0L; count++) { + if (buffer.available() > SftpConstants.MAX_SFTP_MESSAGE_LENGTH) { + throw new IllegalArgumentException( + "Too many file hashes. Use a larger block size or a smaller file chunk. (Block size = " + + blockSize + ", chunk size = " + originalLength + ')'); } - - byte[] hashValue = digest.digest(); + long toHash = Math.min(blockSize, effectiveLength); + long hashed = hashOneBlock(digest, channel, digestBuf, wb, toHash); + if (hashed == 0 && channel.position() >= startOffset) { + // Last block had exhausted the file; no need to write a digest for a zero-length input. + break; + } else if (hashed < toHash) { + // Hashed the last bit, then hit (early) EOF: we're done once we've written the last digest + effectiveLength = 0; + } else { + effectiveLength -= toHash; + } + byte[] hashValue = digest.digest(); // NOTE: this also resets the hash for the next read if (traceEnabled) { - log.trace("doCheckFileHash({})[{}] offset={}, length={} - algo={}, hash={}", - session, file, startOffset, length, - digest.getAlgorithm(), BufferUtils.toHex(':', hashValue)); + log.trace("doCheckFileHash({})({})[{}] offset={}, length={} - algo={}, hash={}", session, count, file, + startOffset, length, digest.getAlgorithm(), BufferUtils.toHex(':', hashValue)); } buffer.putBytes(hashValue); - } else { - for (int count = 0; effectiveLength > 0L; count++) { - int remainLen = Math.min(digestBuf.length, (int) effectiveLength); - ByteBuffer bb = wb; - if (remainLen < digestBuf.length) { - bb = ByteBuffer.wrap(digestBuf, 0, remainLen); - } - bb.clear(); // prepare for next read + } - int readLen = channel.read(bb); - if (readLen < 0) { - break; - } + accessor.closeFile(this, null, file, null, channel, Collections.emptySet()); + } + } - effectiveLength -= readLen; - digest.update(digestBuf, 0, readLen); + private long hashOneBlock( + Digest digest, SeekableByteChannel channel, byte[] digestBuf, ByteBuffer fullBuffer, + long blockSize) throws Exception { + long effectiveLength = blockSize; + long hashed = 0; + while (effectiveLength > 0L) { + int remainLen = (int) Math.min(digestBuf.length, effectiveLength); + ByteBuffer bb = fullBuffer; + if (remainLen < digestBuf.length) { + bb = ByteBuffer.wrap(digestBuf, 0, remainLen); + } + bb.clear(); // prepare for next read - byte[] hashValue = digest.digest(); // NOTE: this also resets the hash for the next read - if (traceEnabled) { - log.trace("doCheckFileHash({})({})[{}] offset={}, length={} - algo={}, hash={}", - session, file, count, startOffset, length, - digest.getAlgorithm(), BufferUtils.toHex(':', hashValue)); - } - buffer.putBytes(hashValue); - } + int readLen = channel.read(bb); + if (readLen < 0) { + break; } - accessor.closeFile(this, null, file, null, channel, Collections.emptySet()); + effectiveLength -= readLen; + digest.update(digestBuf, 0, readLen); + hashed += readLen; } + return hashed; } protected void doMD5Hash(Buffer buffer, int id, String targetType) throws IOException { @@ -1164,11 +1168,6 @@ public abstract class AbstractSftpSubsystemHelper return hashValue; } - protected abstract void doCheckFileHash( - int id, String targetType, String target, Collection<String> algos, - long startOffset, long length, int blockSize, Buffer buffer) - throws Exception; - protected void doReadLink(Buffer buffer, int id) throws IOException { String path = buffer.getString(); Map.Entry<Path, String> link;
