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
