You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mina.apache.org by GitBox <gi...@apache.org> on 2022/12/22 13:39:08 UTC
[GitHub] [mina-sshd] AricBear opened a new issue, #301: Get sender from cmd SSH_MSG_CHANNEL_OPEN_CONFIRMATION wrong could result exception
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-unsubscribe@mina.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
[GitHub] [mina-sshd] tomaswolf closed issue #301: Get sender from cmd SSH_MSG_CHANNEL_OPEN_CONFIRMATION wrong could result exception
Posted by GitBox <gi...@apache.org>.
tomaswolf closed issue #301: Get sender from cmd SSH_MSG_CHANNEL_OPEN_CONFIRMATION wrong could result exception
URL: https://github.com/apache/mina-sshd/issues/301
--
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-unsubscribe@mina.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org
[GitHub] [mina-sshd] tomaswolf commented on issue #301: Get sender from cmd SSH_MSG_CHANNEL_OPEN_CONFIRMATION wrong could result exception
Posted by GitBox <gi...@apache.org>.
tomaswolf commented on issue #301:
URL: https://github.com/apache/mina-sshd/issues/301#issuecomment-1362858864
This is a duplicate of #300.
--
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-unsubscribe@mina.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@mina.apache.org
For additional commands, e-mail: dev-help@mina.apache.org