From 2cbbc3a303d1d4d3e8ddcfdc5452e8b192f582ea Mon Sep 17 00:00:00 2001 From: GT610 Date: Wed, 1 Jul 2026 10:39:11 +0800 Subject: [PATCH 1/2] fix: harden dynamic forwarding, ssh-agent, and EC key parsing Dynamic SOCKS forwarding: - Preserve half-close semantics while streaming: client EOF closes the remote sink and remote EOF destroys the local socket instead of treating EOF as a full connection close immediately. - Guard against concurrent dial attempts for the same SOCKS request. - Cancel the handshake timer before dialing and clean up a tunnel if the connection was closed while dial was in flight. - Tolerate malformed UTF-8 domain names in SOCKS requests and cap handshake buffering at 32 KiB. SSH agent: - Validate that fallback RSA signing produces an SSHRsaSignature with the requested signature type. - Reject zero-length and oversized agent frames to avoid unbounded buffering. EC private keys: - Parse EC PRIVATE KEY curve OIDs when present. - Validate embedded EC public points by deriving the expected public key from the private scalar. - Preserve UnsupportedError for unsupported key types/curves instead of wrapping it as SSHKeyDecodeError. - Decode OpenSSH key comments with allowMalformed to tolerate malformed comments. Adds protocol tests for the dynamic forward hardening and ssh-agent frame size checks. --- lib/src/dynamic_forward_io.dart | 62 +++++++++++++---- lib/src/ssh_agent.dart | 20 +++++- lib/src/ssh_key_pair.dart | 51 ++++++++++++-- test/src/socket/dynamic_forward_io_test.dart | 63 +++++++++++++++++ test/src/ssh_agent_test.dart | 72 ++++++++++++++++++++ 5 files changed, 248 insertions(+), 20 deletions(-) diff --git a/lib/src/dynamic_forward_io.dart b/lib/src/dynamic_forward_io.dart index 191b334..128067c 100644 --- a/lib/src/dynamic_forward_io.dart +++ b/lib/src/dynamic_forward_io.dart @@ -114,20 +114,39 @@ class _SocksConnection { StreamSubscription? _remoteSub; Timer? _handshakeTimer; bool _closed = false; + bool _dialing = false; _SocksState _state = _SocksState.greeting; void start() { - _handshakeTimer = Timer(options.handshakeTimeout, () async { - _sendReply(_SocksReply.ttlExpired); - await close(); - }); - _clientSub = _client.listen( _onClientData, - onDone: close, + onDone: _handleClientEOF, onError: (_, __) => close(), cancelOnError: true, ); + + _handshakeTimer = Timer(options.handshakeTimeout, () async { + _sendReply(_SocksReply.ttlExpired); + await close(); + }); + } + + void _handleClientEOF() { + if (_state == _SocksState.streaming) { + _remote?.sink.close(); + _clientSub?.cancel(); + } else { + close(); + } + } + + void _handleRemoteEOF() { + if (_state == _SocksState.streaming) { + _client.destroy(); + _remoteSub?.cancel(); + } else { + close(); + } } Future close() async { @@ -152,9 +171,8 @@ class _SocksConnection { return; } - _buffer.add(chunk); - try { + _buffer.add(chunk); await _consumeHandshake(); } catch (_) { await close(); @@ -169,41 +187,55 @@ class _SocksConnection { } if (_state == _SocksState.request) { + if (_dialing) return; final target = _parseConnectRequest(); if (target == null) return; + _dialing = true; if (filter != null && !filter!(target.host, target.port)) { _sendReply(_SocksReply.connectionNotAllowed); + _dialing = false; await close(); return; } if (!canOpenTunnel()) { _sendReply(_SocksReply.connectionRefused); + _dialing = false; await close(); return; } + _handshakeTimer?.cancel(); + _handshakeTimer = null; + try { _remote = await dial(target.host, target.port).timeout( options.connectTimeout, ); } catch (_) { _sendReply(_SocksReply.hostUnreachable); + _dialing = false; await close(); return; } + _dialing = false; + + if (_closed) { + _remote?.destroy(); + _remote = null; + return; + } + _remoteSub = _remote!.stream.listen( _client.add, - onDone: close, + onDone: _handleRemoteEOF, onError: (_, __) => close(), cancelOnError: true, ); _sendReply(_SocksReply.succeeded); - _handshakeTimer?.cancel(); - _handshakeTimer = null; _state = _SocksState.streaming; final pending = _buffer.takeAll(); @@ -288,7 +320,7 @@ class _SocksConnection { if (atyp == 0x03) { final length = request[4]; final bytes = request.sublist(5, 5 + length); - return utf8.decode(bytes); + return utf8.decode(bytes, allowMalformed: true); } final raw = request.sublist(4, 20); @@ -316,12 +348,18 @@ class _SocksConnection { } class _ByteBuffer { + static const kMaxHandshakeSize = 32768; + final _data = []; int _offset = 0; int get length => _data.length - _offset; void add(List chunk) { + if (length + chunk.length > kMaxHandshakeSize) { + throw StateError( + 'Handshake buffer overflow: $length + ${chunk.length} > $kMaxHandshakeSize'); + } _data.addAll(chunk); } diff --git a/lib/src/ssh_agent.dart b/lib/src/ssh_agent.dart index 7e48fba..2456a8a 100644 --- a/lib/src/ssh_agent.dart +++ b/lib/src/ssh_agent.dart @@ -95,7 +95,16 @@ class SSHKeyPairAgent implements SSHAgentHandler { ) { final key = _rsaKeyFrom(identity); if (key == null) { - return identity.sign(data) as SSHRsaSignature; + final signature = identity.sign(data); + if (signature is SSHRsaSignature) { + if (signature.type != signatureType) { + throw StateError( + 'RSA signature type mismatch: requested $signatureType but identity produced ${signature.type}'); + } + return signature; + } + throw StateError( + 'RSA signing requested but identity produced non-RSA signature: ${signature.runtimeType}'); } final signer = _rsaSignerFor(signatureType); @@ -154,6 +163,8 @@ class SSHKeyPairAgent implements SSHAgentHandler { } class SSHAgentChannel { + static const maxFrameSize = 256 * 1024; + SSHAgentChannel(this._channel, this._handler, {this.printDebug}) { _subscription = _channel.stream.listen( _handleData, @@ -188,6 +199,13 @@ class SSHAgentChannel { Future _processQueue() async { while (_buffer.length >= 4) { final length = ByteData.sublistView(_buffer, 0, 4).getUint32(0); + if (length == 0 || length > maxFrameSize) { + printDebug + ?.call('SSH agent: invalid frame length $length, closing channel'); + _channel.destroy(); + _buffer = Uint8List(0); + return; + } if (_buffer.length < 4 + length) return; final payload = _buffer.sublist(4, 4 + length); _buffer = _buffer.sublist(4 + length); diff --git a/lib/src/ssh_key_pair.dart b/lib/src/ssh_key_pair.dart index fef8782..40ed6fd 100644 --- a/lib/src/ssh_key_pair.dart +++ b/lib/src/ssh_key_pair.dart @@ -239,8 +239,14 @@ class OpenSSHKeyPairs { final key = Uint8List.view(kdfHash.buffer, 0, cipher.keySize); final iv = Uint8List.view(kdfHash.buffer, cipher.keySize, cipher.ivSize); - final decryptCipher = cipher.createCipher(key, iv, forEncryption: false); - return decryptCipher.processAll(blob); + + try { + final decryptCipher = + cipher.createCipher(key, iv, forEncryption: false); + return decryptCipher.processAll(blob); + } catch (e) { + throw SSHKeyDecryptError('Failed to decrypt private key', e); + } } @override @@ -339,7 +345,7 @@ class OpenSSHRsaKeyPair with OpenSSHKeyPair { final iqmp = reader.readMpint(); final p = reader.readMpint(); final q = reader.readMpint(); - final comment = reader.readUtf8(); + final comment = reader.readUtf8(allowMalformed: true); return OpenSSHRsaKeyPair(n, e, d, iqmp, p, q, comment); } @@ -397,7 +403,7 @@ class OpenSSHEd25519KeyPair with OpenSSHKeyPair { factory OpenSSHEd25519KeyPair.readFrom(SSHMessageReader reader) { final publicKey = reader.readString(); final privateKey = reader.readString(); - final comment = reader.readUtf8(); + final comment = reader.readUtf8(allowMalformed: true); return OpenSSHEd25519KeyPair(publicKey, privateKey, comment); } @@ -446,7 +452,7 @@ class OpenSSHEcdsaKeyPair with OpenSSHKeyPair { final curve = reader.readUtf8(); final q = reader.readString(); final d = reader.readMpint(); - final comment = reader.readUtf8(); + final comment = reader.readUtf8(allowMalformed: true); return OpenSSHEcdsaKeyPair(curve, q, d, comment); } @@ -538,6 +544,8 @@ class RsaKeyPair { try { return RsaPrivateKey.decode(keyBlob); + } on UnsupportedError { + rethrow; } catch (e) { throw SSHKeyDecodeError('Failed to decode private key', e); } @@ -755,6 +763,8 @@ class EcKeyPair { try { return _decodeLegacyEcPrivateKey(keyBlob); + } on UnsupportedError { + rethrow; } catch (e) { throw SSHKeyDecodeError('Failed to decode private key', e); } @@ -772,10 +782,21 @@ class EcKeyPair { final d = decodeBigIntWithSign(1, privateKeyOctets); Uint8List? publicPoint; + String? curveId; for (var i = 2; i < sequence.elements.length; i++) { final element = sequence.elements[i]; - if (element.tag == 0xA1) { + if (element.tag == 0xA0) { + final inner = ASN1Parser(element.valueBytes()).nextObject(); + if (inner is ASN1ObjectIdentifier && inner.identifier != null) { + final oid = inner.identifier!; + curveId = _curveIdFromOid(oid); + if (curveId == null) { + throw UnsupportedError( + 'Unsupported EC PRIVATE KEY curve OID: $oid'); + } + } + } else if (element.tag == 0xA1) { final inner = ASN1Parser(element.valueBytes()).nextObject(); if (inner is ASN1BitString) { publicPoint = inner.contentBytes(); @@ -783,17 +804,33 @@ class EcKeyPair { } } - final curveId = + curveId ??= _inferCurveId(publicPoint?.length ?? 0, privateKeyOctets.length); if (curveId == null) { throw UnsupportedError('Unsupported EC PRIVATE KEY curve'); } + if (publicPoint != null) { + final expectedPublicPoint = _derivePublicPoint(curveId, d); + if (publicPoint.length != expectedPublicPoint.length || + !publicPoint.equals(expectedPublicPoint)) { + throw UnsupportedError( + 'EC PRIVATE KEY public point does not match curve $curveId'); + } + } + final q = publicPoint ?? _derivePublicPoint(curveId, d); return OpenSSHEcdsaKeyPair(curveId, q, d, ''); } + String? _curveIdFromOid(String oid) { + if (oid == '1.2.840.10045.3.1.7') return 'nistp256'; + if (oid == '1.3.132.0.34') return 'nistp384'; + if (oid == '1.3.132.0.35') return 'nistp521'; + return null; + } + String? _inferCurveId(int publicPointLength, int privateKeyLength) { if (publicPointLength == 65 || privateKeyLength == 32) { return 'nistp256'; diff --git a/test/src/socket/dynamic_forward_io_test.dart b/test/src/socket/dynamic_forward_io_test.dart index f52fe7d..d692795 100644 --- a/test/src/socket/dynamic_forward_io_test.dart +++ b/test/src/socket/dynamic_forward_io_test.dart @@ -369,6 +369,69 @@ void main() { expect(dialedHosts[0], '192.168.1.2'); expect(dialedHosts[1], contains(':')); }); + + test('closes remote sink when client EOF arrives during streaming', + () async { + late _DialedTunnel dialed; + + final forward = await startDynamicForward( + bindHost: '127.0.0.1', + bindPort: 0, + options: const SSHDynamicForwardOptions(), + dial: (_, __) async { + dialed = _DialedTunnel.create(); + return dialed.channel; + }, + ); + + final client = await Socket.connect(forward.host, forward.port); + final incoming = client.asBroadcastStream(); + addTearDown(() async { + await client.close(); + await forward.close(); + dialed.dispose(); + }); + + await _sendGreeting(client, incoming); + final reply = + await _sendConnectDomain(client, incoming, 'example.com', 443); + expect(reply[1], 0x00); + + // Send some data then close client side (half-close / EOF). + client.add(utf8.encode('data')); + await client.close(); + await Future.delayed(const Duration(milliseconds: 30)); + }); + + test('handles handshake buffer overflow gracefully', () async { + final forward = await startDynamicForward( + bindHost: '127.0.0.1', + bindPort: 0, + options: const SSHDynamicForwardOptions(), + dial: (_, __) async => _DialedTunnel.create().channel, + ); + addTearDown(() => forward.close()); + + final client = await Socket.connect(forward.host, forward.port); + addTearDown(() => client.close()); + + // Send a valid greeting but then flood the handshake buffer beyond + // kMaxHandshakeSize (32768). The server should close the connection + // rather than keep buffering indefinitely. + await _sendGreeting(client, client.asBroadcastStream()); + final huge = Uint8List(33000); + client.add(huge); + + // Give the server time to detect the overflow and close. The test + // passes if the server does not crash — the forward is still usable + // for a new connection after the overflow victim is cleaned up. + await Future.delayed(const Duration(milliseconds: 100)); + + // Verify the forward still accepts new connections. + final client2 = await Socket.connect(forward.host, forward.port); + addTearDown(() => client2.close()); + await _sendGreeting(client2, client2.asBroadcastStream()); + }); }); } diff --git a/test/src/ssh_agent_test.dart b/test/src/ssh_agent_test.dart index 6a24535..a131dbe 100644 --- a/test/src/ssh_agent_test.dart +++ b/test/src/ssh_agent_test.dart @@ -290,4 +290,76 @@ void main() { controller.destroy(); }); + + test('SSHAgentChannel closes on invalid frame length (zero)', () async { + final handler = _RecordingAgentHandler(Uint8List.fromList([1])); + + final controller = SSHChannelController( + localId: 1, + localMaximumPacketSize: 1024, + localInitialWindowSize: 1024, + remoteId: 2, + remoteMaximumPacketSize: 1024, + remoteInitialWindowSize: 1024, + sendMessage: (_) {}, + ); + + SSHAgentChannel( + controller.channel, + handler, + printDebug: (_) {}, + ); + + // Frame with length = 0 (invalid). + final zeroFrame = Uint8List.fromList([0, 0, 0, 0]); + controller.handleMessage( + SSH_Message_Channel_Data( + recipientChannel: controller.localId, + data: zeroFrame, + ), + ); + + await Future.delayed(Duration.zero); + + // The channel should have been destroyed, no requests processed. + expect(handler.requests, isEmpty); + + controller.destroy(); + }); + + test('SSHAgentChannel closes on frame length exceeding maxFrameSize', + () async { + final handler = _RecordingAgentHandler(Uint8List.fromList([1])); + + final controller = SSHChannelController( + localId: 1, + localMaximumPacketSize: 1024, + localInitialWindowSize: 1024, + remoteId: 2, + remoteMaximumPacketSize: 1024, + remoteInitialWindowSize: 1024, + sendMessage: (_) {}, + ); + + SSHAgentChannel( + controller.channel, + handler, + printDebug: (_) {}, + ); + + // Frame claiming a length > maxFrameSize (256 * 1024 = 262144). + final oversizedFrame = Uint8List.fromList([0, 4, 0, 1]); + controller.handleMessage( + SSH_Message_Channel_Data( + recipientChannel: controller.localId, + data: oversizedFrame, + ), + ); + + await Future.delayed(Duration.zero); + + expect(handler.requests, isEmpty); + + controller.destroy(); + }); } From 4e707e1af1c092eae877a69544de78c25c1c3e72 Mon Sep 17 00:00:00 2001 From: GT610 Date: Wed, 1 Jul 2026 10:41:43 +0800 Subject: [PATCH 2/2] Format --- lib/src/ssh_key_pair.dart | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/src/ssh_key_pair.dart b/lib/src/ssh_key_pair.dart index 40ed6fd..096ecea 100644 --- a/lib/src/ssh_key_pair.dart +++ b/lib/src/ssh_key_pair.dart @@ -241,8 +241,7 @@ class OpenSSHKeyPairs { final iv = Uint8List.view(kdfHash.buffer, cipher.keySize, cipher.ivSize); try { - final decryptCipher = - cipher.createCipher(key, iv, forEncryption: false); + final decryptCipher = cipher.createCipher(key, iv, forEncryption: false); return decryptCipher.processAll(blob); } catch (e) { throw SSHKeyDecryptError('Failed to decrypt private key', e);