AricBear opened a new issue, #301: URL: https://github.com/apache/mina-sshd/issues/301
### Version 2.9.2 ### Bug description While opening a channel, mina deals cmd of SSH_MSG_CHANNEL_OPEN_CONFIRMATION in org.apache.sshd.common.session.helpers.AbstractConnectionService#channelOpenConfirmation. ```java public void channelOpenConfirmation(Buffer buffer) throws IOException { Channel channel = getChannel(SshConstants.SSH_MSG_CHANNEL_OPEN_CONFIRMATION, buffer); if (channel == null) { return; // debug breakpoint } int sender = buffer.getInt(); long rwsize = buffer.getUInt(); long rmpsize = buffer.getUInt(); if (log.isDebugEnabled()) { log.debug("channelOpenConfirmation({}) SSH_MSG_CHANNEL_OPEN_CONFIRMATION sender={}, window-size={}, packet-size={}", channel, sender, rwsize, rmpsize); } /* * NOTE: the 'sender' of the SSH_MSG_CHANNEL_OPEN_CONFIRMATION is the recipient on the client side - see rfc4254 * section 5.1: * * 'sender channel' is the channel number allocated by the other side * * in our case, the server */ channel.handleOpenSuccess(sender, rwsize, rmpsize, buffer); } ``` mina defines sender as int( `int sender = buffer.getInt();` ), while sender should be UINT32. Therefore , here could be a situation, when sender is bigger than 0x7fffffff, and turns to be a minus int, and then be set as channel's recipient. When mina uses channel's recipient later, e.g. org.apache.sshd.common.channel.ChannelAsyncOutputStream#createSendBuffer ```java protected Buffer createSendBuffer(Buffer buffer, Channel channel, int length) { SessionContext.validateSessionPayloadSize(length, "Invalid send buffer length: %d"); Session s = channel.getSession(); Buffer buf = s.createBuffer(cmd, length + 12); buf.putUInt(channel.getRecipient()); if (cmd == SshConstants.SSH_MSG_CHANNEL_EXTENDED_DATA) { buf.putUInt(SshConstants.SSH_EXTENDED_DATA_STDERR); } buf.putUInt(length); buf.putRawBytes(buffer.array(), buffer.rpos(), length); buffer.rpos(buffer.rpos() + length); return buf; } ``` when put channel's recipient into buffer, `putUInt(long i)` checks input i, if i is a minus number, an IllegalArgumentException would be thrown, and the channel would be closed. I searched git log, found out this problem is related to [SSHD-1244]. Lyor Goldstein changed lots of int variables into long to hold uint32 values, but unfortunately he missed some line. Line `int sender = buffer.getInt();` in `channelOpenConfirmation()` is untouched while `org.apache.sshd.common.session.helpers.AbstractConnectionService#channelOpen` is correctly changed. Not sure whether there were more left unmodified, since there were lots code change in [SSHD-1244]. We should review code to make sure. ### Actual behavior I use mina sshd as server. A client is connecting to server, send SSH_MSG_CHANNEL_OPEN_CONFIRMATION while sender is something like 0xFF010015, then exception is thrown , session is closed. ### Expected behavior It should connect like any other client. ### Relevant log output ```Shell 2022-12-19T15:32:57,781 | DEBUG | MinaProcessor-11 | ServerSessionImpl |doHandleMessage() 551 | org.apache.sshd.common.session.helpers.AbstractSession | | | | doHandleMessage(ServerSessionImpl[user@/120.126.12.100:29030]) process #8 SSH_MSG_CHANNEL_OPEN_CONFIRMATION 2022-12-19T15:32:57,781 | DEBUG | MinaProcessor-11 | ServerConnectionService |channelOpenConfirmation() 534 | org.apache.sshd.common.session.helpers.AbstractConnectionService | | | | channelOpenConfirmation(TcpipClientChannel[id=0, recipient=-1]-ServerSessionImpl[user@/120.126.12.100:29030]) SSH_MSG_CHANNEL_OPEN_CONFIRMATION sender=-2147418110, window-size=131072, packet-size=32768 2022-12-19T15:32:57,781 | DEBUG | MinaProcessor-11 | TcpipClientChannel |setRecipient() 172 | org.apache.sshd.common.channel.AbstractChannel | | | | setRecipient(TcpipClientChannel[id=0, recipient=-1]-ServerSessionImpl[user@/120.126.12.100:29030]) recipient=-2147418110 java.lang.IllegalArgumentException: Invalid UINT32 value: -2147418097 at org.apache.sshd.common.util.ValidateUtils.createFormattedException(ValidateUtils.java:213) ~[sshd-common-2.9.2.jar:2.9.2] at org.apache.sshd.common.util.ValidateUtils.throwIllegalArgumentException(ValidateUtils.java:179) ~[sshd-common-2.9.2.jar:2.9.2] at org.apache.sshd.common.util.ValidateUtils.checkTrue(ValidateUtils.java:162) ~[sshd-common-2.9.2.jar:2.9.2] at org.apache.sshd.common.util.buffer.BufferUtils.validateUint32Value(BufferUtils.java:701) ~[sshd-common-2.9.2.jar:2.9.2] at org.apache.sshd.common.util.buffer.Buffer.putUInt(Buffer.java:720) ~[sshd-common-2.9.2.jar:2.9.2] at org.apache.sshd.common.channel.ChannelAsyncOutputStream.createSendBuffer(ChannelAsyncOutputStream.java:393) ~[sshd-core-2.9.2.jar:2.9.2] at org.apache.sshd.common.channel.ChannelAsyncOutputStream.writePacket(ChannelAsyncOutputStream.java:338) ~[sshd-core-2.9.2.jar:2.9.2] at org.apache.sshd.common.channel.ChannelAsyncOutputStream.doWriteIfPossible(ChannelAsyncOutputStream.java:215) ~[sshd-core-2.9.2.jar:2.9.2] at org.apache.sshd.common.channel.ChannelAsyncOutputStream.writeBuffer(ChannelAsyncOutputStream.java:110) ~[sshd-core-2.9.2.jar:2.9.2] at org.apache.sshd.common.forward.DefaultForwarder$StaticIoHandler.lambda$messageReceived$2(DefaultForwarder.java:1063) ~[sshd-core-2.9.2.jar:2.9.2] at org.apache.sshd.common.util.threads.ThreadUtils.runAsInternal(ThreadUtils.java:68) ~[sshd-common-2.9.2.jar:2.9.2] at org.apache.sshd.common.forward.DefaultForwarder$StaticIoHandler.messageReceived(DefaultForwarder.java:1063) ~[sshd-core-2.9.2.jar:2.9.2] at org.apache.sshd.mina.MinaService.messageReceived(MinaService.java:156) ~[sshd-mina-2.9.2.jar:2.9.2] at org.apache.mina.core.filterchain.DefaultIoFilterChain$TailFilter.messageReceived(DefaultIoFilterChain.java:1015) ~[mina-core-2.1.6.jar:?] at org.apache.mina.core.filterchain.DefaultIoFilterChain.callNextMessageReceived(DefaultIoFilterChain.java:650) ~[mina-core-2.1.6.jar:?] at org.apache.mina.core.filterchain.DefaultIoFilterChain.access$1300(DefaultIoFilterChain.java:49) ~[mina-core-2.1.6.jar:?] at org.apache.mina.core.filterchain.DefaultIoFilterChain$EntryImpl$1.messageReceived(DefaultIoFilterChain.java:1128) ~[mina-core-2.1.6.jar:?] at org.apache.mina.core.filterchain.IoFilterAdapter.messageReceived(IoFilterAdapter.java:122) ~[mina-core-2.1.6.jar:?] at org.apache.mina.core.filterchain.DefaultIoFilterChain.callNextMessageReceived(DefaultIoFilterChain.java:650) ~[mina-core-2.1.6.jar:?] at org.apache.mina.core.filterchain.DefaultIoFilterChain.fireMessageReceived(DefaultIoFilterChain.java:643) ~[mina-core-2.1.6.jar:?] at org.apache.mina.core.polling.AbstractPollingIoProcessor.read(AbstractPollingIoProcessor.java:539) ~[mina-core-2.1.6.jar:?] at org.apache.mina.core.polling.AbstractPollingIoProcessor.access$1200(AbstractPollingIoProcessor.java:68) ~[mina-core-2.1.6.jar:?] at org.apache.mina.core.polling.AbstractPollingIoProcessor$Processor.process(AbstractPollingIoProcessor.java:1224) ~[mina-core-2.1.6.jar:?] at org.apache.mina.core.polling.AbstractPollingIoProcessor$Processor.process(AbstractPollingIoProcessor.java:1213) ~[mina-core-2.1.6.jar:?] at org.apache.mina.core.polling.AbstractPollingIoProcessor$Processor.run(AbstractPollingIoProcessor.java:683) ~[mina-core-2.1.6.jar:?] ``` ### Other information This bug is not existed in mina sshd 2.8.0. In 2.8.0, everything is hold by int, and there are no value check when putting in buffer, so even if sender is minus, that doesn't matter. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org For additional commands, e-mail: dev-h...@mina.apache.org