Skip to content

Commit 2a83ee9

Browse files
loull521liushi.lll
andauthored
fix: prevent integer overflow in RPC command decoder length calculation (#382)
* fix: prevent integer overflow in RPC command decoder length calculation Add RpcCommandDecoderLengthValidator to validate classLen, headerLen and contentLen before summing them up. Negative values and totals exceeding Integer.MAX_VALUE are rejected with CodecException, preventing a crafted packet from triggering integer overflow and potential DOS. Applied to both RpcCommandDecoder (v1) and RpcCommandDecoderV2, covering request and response paths. TVM #2923220 * refine: move arrive() after length validation, add response test cases - Move ThreadLocalArriveTimeHolder.arrive() after validator to avoid ThreadLocal leak when CodecException is thrown for malformed packets - Add v1 response overflow rejection test - Add v1/v2 response normal decode tests - Use StandardCharsets.UTF_8 in test byte literals * chore: bump version to 1.6.13 --------- Co-authored-by: liushi.lll <liushi.lll@antgroup.com>
1 parent 260afeb commit 2a83ee9

6 files changed

Lines changed: 289 additions & 24 deletions

File tree

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,3 +21,4 @@ coverage-report
2121
test-output
2222
*.txt
2323
.DS_Store
24+
AGENTS.md

pom.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
<modelVersion>4.0.0</modelVersion>
2323
<groupId>com.alipay.sofa</groupId>
2424
<artifactId>bolt</artifactId>
25-
<version>1.6.12</version>
25+
<version>1.6.13</version>
2626
<packaging>jar</packaging>
2727

2828
<name>${project.groupId}:${project.artifactId}</name>

src/main/java/com/alipay/remoting/rpc/protocol/RpcCommandDecoder.java

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,9 @@ public void decode(ChannelHandlerContext ctx, ByteBuf in, List<Object> out) thro
100100
byte[] content = null;
101101
Channel channel = ctx.channel();
102102
ThreadLocalArriveTimeHolder.arrive(channel, requestId);
103-
if (in.readableBytes() >= classLen + headerLen + contentLen) {
103+
int frameLength = RpcCommandDecoderLengthValidator
104+
.validateAndGetTotalLength(classLen, headerLen, contentLen, 0);
105+
if (in.readableBytes() >= frameLength) {
104106
if (classLen > 0) {
105107
clazz = new byte[classLen];
106108
in.readBytes(clazz);
@@ -152,7 +154,9 @@ public void decode(ChannelHandlerContext ctx, ByteBuf in, List<Object> out) thro
152154
byte[] clazz = null;
153155
byte[] header = null;
154156
byte[] content = null;
155-
if (in.readableBytes() >= classLen + headerLen + contentLen) {
157+
int frameLength = RpcCommandDecoderLengthValidator
158+
.validateAndGetTotalLength(classLen, headerLen, contentLen, 0);
159+
if (in.readableBytes() >= frameLength) {
156160
if (classLen > 0) {
157161
clazz = new byte[classLen];
158162
in.readBytes(clazz);
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
package com.alipay.remoting.rpc.protocol;
18+
19+
import com.alipay.remoting.exception.CodecException;
20+
21+
final class RpcCommandDecoderLengthValidator {
22+
23+
private RpcCommandDecoderLengthValidator() {
24+
}
25+
26+
static int validateAndGetTotalLength(short classLen, short headerLen, int contentLen,
27+
int trailerLen) throws CodecException {
28+
if (classLen < 0 || headerLen < 0 || contentLen < 0 || trailerLen < 0) {
29+
throw new CodecException("Illegal RPC command length: classLen=" + classLen
30+
+ ", headerLen=" + headerLen + ", contentLen=" + contentLen
31+
+ ", trailerLen=" + trailerLen);
32+
}
33+
34+
long totalLength = (long) classLen + headerLen + contentLen + trailerLen;
35+
if (totalLength > Integer.MAX_VALUE) {
36+
throw new CodecException("RPC command length exceeds integer range: " + totalLength);
37+
}
38+
return (int) totalLength;
39+
}
40+
}

src/main/java/com/alipay/remoting/rpc/protocol/RpcCommandDecoderV2.java

Lines changed: 15 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -105,22 +105,19 @@ public void decode(ChannelHandlerContext ctx, ByteBuf in, List<Object> out) thro
105105
byte[] header = null;
106106
byte[] content = null;
107107

108-
Channel channel = ctx.channel();
109-
ThreadLocalArriveTimeHolder.arrive(channel, requestId);
110-
111-
// decide the at-least bytes length for each version
112-
int lengthAtLeastForV1 = classLen + headerLen + contentLen;
113108
boolean crcSwitchOn = ProtocolSwitch.isOn(
114109
ProtocolSwitch.CRC_SWITCH_INDEX, protocolSwitchValue);
115-
int lengthAtLeastForV2 = classLen + headerLen + contentLen;
116-
if (crcSwitchOn) {
117-
lengthAtLeastForV2 += 4;// crc int
118-
}
110+
int frameLength = RpcCommandDecoderLengthValidator
111+
.validateAndGetTotalLength(classLen, headerLen, contentLen,
112+
version == RpcProtocolV2.PROTOCOL_VERSION_2 && crcSwitchOn ? 4
113+
: 0);
114+
115+
Channel channel = ctx.channel();
116+
ThreadLocalArriveTimeHolder.arrive(channel, requestId);
119117

120118
// continue read
121-
if ((version == RpcProtocolV2.PROTOCOL_VERSION_1 && in.readableBytes() >= lengthAtLeastForV1)
122-
|| (version == RpcProtocolV2.PROTOCOL_VERSION_2 && in
123-
.readableBytes() >= lengthAtLeastForV2)) {
119+
if ((version == RpcProtocolV2.PROTOCOL_VERSION_1 || version == RpcProtocolV2.PROTOCOL_VERSION_2)
120+
&& in.readableBytes() >= frameLength) {
124121
if (classLen > 0) {
125122
clazz = new byte[classLen];
126123
in.readBytes(clazz);
@@ -180,19 +177,16 @@ public void decode(ChannelHandlerContext ctx, ByteBuf in, List<Object> out) thro
180177
byte[] header = null;
181178
byte[] content = null;
182179

183-
// decide the at-least bytes length for each version
184-
int lengthAtLeastForV1 = classLen + headerLen + contentLen;
185180
boolean crcSwitchOn = ProtocolSwitch.isOn(
186181
ProtocolSwitch.CRC_SWITCH_INDEX, protocolSwitchValue);
187-
int lengthAtLeastForV2 = classLen + headerLen + contentLen;
188-
if (crcSwitchOn) {
189-
lengthAtLeastForV2 += 4;// crc int
190-
}
182+
int frameLength = RpcCommandDecoderLengthValidator
183+
.validateAndGetTotalLength(classLen, headerLen, contentLen,
184+
version == RpcProtocolV2.PROTOCOL_VERSION_2 && crcSwitchOn ? 4
185+
: 0);
191186

192187
// continue read
193-
if ((version == RpcProtocolV2.PROTOCOL_VERSION_1 && in.readableBytes() >= lengthAtLeastForV1)
194-
|| (version == RpcProtocolV2.PROTOCOL_VERSION_2 && in
195-
.readableBytes() >= lengthAtLeastForV2)) {
188+
if ((version == RpcProtocolV2.PROTOCOL_VERSION_1 || version == RpcProtocolV2.PROTOCOL_VERSION_2)
189+
&& in.readableBytes() >= frameLength) {
196190
if (classLen > 0) {
197191
clazz = new byte[classLen];
198192
in.readBytes(clazz);
Lines changed: 226 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,226 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
package com.alipay.remoting.rpc.protocol;
18+
19+
import java.nio.charset.StandardCharsets;
20+
import java.util.ArrayList;
21+
import java.util.List;
22+
23+
import com.alipay.remoting.exception.CodecException;
24+
import com.alipay.remoting.rpc.RpcCommandType;
25+
import io.netty.buffer.ByteBuf;
26+
import io.netty.buffer.Unpooled;
27+
import io.netty.channel.Channel;
28+
import io.netty.channel.ChannelHandlerContext;
29+
import org.junit.Assert;
30+
import org.junit.Test;
31+
import org.mockito.Mockito;
32+
33+
public class RpcCommandDecoderTest {
34+
35+
@Test(expected = CodecException.class)
36+
public void rejectsNegativeClassLengthInV1Request() throws Exception {
37+
ByteBuf frame = v1RequestFrame((short) -1, (short) 0, 0);
38+
try {
39+
new RpcCommandDecoder().decode(context(), frame, new ArrayList<Object>());
40+
} finally {
41+
frame.release();
42+
}
43+
}
44+
45+
@Test(expected = CodecException.class)
46+
public void rejectsNegativeHeaderLengthInV2Request() throws Exception {
47+
ByteBuf frame = v2RequestFrame((short) 0, (short) -1, 0);
48+
try {
49+
new RpcCommandDecoderV2().decode(context(), frame, new ArrayList<Object>());
50+
} finally {
51+
frame.release();
52+
}
53+
}
54+
55+
@Test(expected = CodecException.class)
56+
public void rejectsNegativeContentLengthInV1Request() throws Exception {
57+
ByteBuf frame = v1RequestFrame((short) 0, (short) 0, -1);
58+
try {
59+
new RpcCommandDecoder().decode(context(), frame, new ArrayList<Object>());
60+
} finally {
61+
frame.release();
62+
}
63+
}
64+
65+
@Test(expected = CodecException.class)
66+
public void rejectsNegativeContentLengthInV2Request() throws Exception {
67+
ByteBuf frame = v2RequestFrame((short) 0, (short) 0, -1);
68+
try {
69+
new RpcCommandDecoderV2().decode(context(), frame, new ArrayList<Object>());
70+
} finally {
71+
frame.release();
72+
}
73+
}
74+
75+
@Test(expected = CodecException.class)
76+
public void rejectsOverflowedTotalLengthInV1Request() throws Exception {
77+
ByteBuf frame = v1RequestFrame((short) 1, (short) 0, Integer.MAX_VALUE);
78+
try {
79+
new RpcCommandDecoder().decode(context(), frame, new ArrayList<Object>());
80+
} finally {
81+
frame.release();
82+
}
83+
}
84+
85+
@Test(expected = CodecException.class)
86+
public void rejectsOverflowedTotalLengthInV2Request() throws Exception {
87+
ByteBuf frame = v2RequestFrame((short) 1, (short) 0, Integer.MAX_VALUE);
88+
try {
89+
new RpcCommandDecoderV2().decode(context(), frame, new ArrayList<Object>());
90+
} finally {
91+
frame.release();
92+
}
93+
}
94+
95+
@Test(expected = CodecException.class)
96+
public void rejectsNegativeClassLengthInV1Response() throws Exception {
97+
ByteBuf frame = v1ResponseFrame((short) -1, (short) 0, 0);
98+
try {
99+
new RpcCommandDecoder().decode(context(), frame, new ArrayList<Object>());
100+
} finally {
101+
frame.release();
102+
}
103+
}
104+
105+
@Test(expected = CodecException.class)
106+
public void rejectsNegativeHeaderLengthInV2Response() throws Exception {
107+
ByteBuf frame = v2ResponseFrame((short) 0, (short) -1, 0);
108+
try {
109+
new RpcCommandDecoderV2().decode(context(), frame, new ArrayList<Object>());
110+
} finally {
111+
frame.release();
112+
}
113+
}
114+
115+
@Test(expected = CodecException.class)
116+
public void rejectsOverflowedTotalLengthInV2Response() throws Exception {
117+
ByteBuf frame = v2ResponseFrame((short) 0, (short) 1, Integer.MAX_VALUE);
118+
try {
119+
new RpcCommandDecoderV2().decode(context(), frame, new ArrayList<Object>());
120+
} finally {
121+
frame.release();
122+
}
123+
}
124+
125+
@Test(expected = CodecException.class)
126+
public void rejectsOverflowedTotalLengthInV1Response() throws Exception {
127+
ByteBuf frame = v1ResponseFrame((short) 0, (short) 1, Integer.MAX_VALUE);
128+
try {
129+
new RpcCommandDecoder().decode(context(), frame, new ArrayList<Object>());
130+
} finally {
131+
frame.release();
132+
}
133+
}
134+
135+
@Test
136+
public void decodesValidV2RequestWithContent() throws Exception {
137+
byte[] body = "hello".getBytes(StandardCharsets.UTF_8);
138+
ByteBuf frame = v2RequestFrame((short) 0, (short) 0, body.length);
139+
frame.writeBytes(body);
140+
List<Object> out = new ArrayList<Object>();
141+
try {
142+
new RpcCommandDecoderV2().decode(context(), frame, out);
143+
} finally {
144+
frame.release();
145+
}
146+
Assert.assertEquals(1, out.size());
147+
}
148+
149+
@Test
150+
public void decodesValidV1RequestWithContent() throws Exception {
151+
byte[] body = "world".getBytes(StandardCharsets.UTF_8);
152+
ByteBuf frame = v1RequestFrame((short) 0, (short) 0, body.length);
153+
frame.writeBytes(body);
154+
List<Object> out = new ArrayList<Object>();
155+
try {
156+
new RpcCommandDecoder().decode(context(), frame, out);
157+
} finally {
158+
frame.release();
159+
}
160+
Assert.assertEquals(1, out.size());
161+
}
162+
163+
@Test
164+
public void decodesValidV1ResponseWithContent() throws Exception {
165+
byte[] body = "resp-v1".getBytes(StandardCharsets.UTF_8);
166+
ByteBuf frame = v1ResponseFrame((short) 0, (short) 0, body.length);
167+
frame.writeBytes(body);
168+
List<Object> out = new ArrayList<Object>();
169+
try {
170+
new RpcCommandDecoder().decode(context(), frame, out);
171+
} finally {
172+
frame.release();
173+
}
174+
Assert.assertEquals(1, out.size());
175+
}
176+
177+
@Test
178+
public void decodesValidV2ResponseWithContent() throws Exception {
179+
byte[] body = "resp-v2".getBytes(StandardCharsets.UTF_8);
180+
ByteBuf frame = v2ResponseFrame((short) 0, (short) 0, body.length);
181+
frame.writeBytes(body);
182+
List<Object> out = new ArrayList<Object>();
183+
try {
184+
new RpcCommandDecoderV2().decode(context(), frame, out);
185+
} finally {
186+
frame.release();
187+
}
188+
Assert.assertEquals(1, out.size());
189+
}
190+
191+
private static ByteBuf v1RequestFrame(short classLen, short headerLen, int contentLen) {
192+
return Unpooled.buffer(RpcProtocol.getRequestHeaderLength())
193+
.writeByte(RpcProtocol.PROTOCOL_CODE).writeByte(RpcCommandType.REQUEST)
194+
.writeShort(RpcCommandCode.RPC_REQUEST.value()).writeByte(1).writeInt(1).writeByte(1)
195+
.writeInt(1000).writeShort(classLen).writeShort(headerLen).writeInt(contentLen);
196+
}
197+
198+
private static ByteBuf v2RequestFrame(short classLen, short headerLen, int contentLen) {
199+
return Unpooled.buffer(RpcProtocolV2.getRequestHeaderLength())
200+
.writeByte(RpcProtocolV2.PROTOCOL_CODE).writeByte(RpcProtocolV2.PROTOCOL_VERSION_1)
201+
.writeByte(RpcCommandType.REQUEST).writeShort(RpcCommandCode.RPC_REQUEST.value())
202+
.writeByte(1).writeInt(1).writeByte(1).writeByte(0).writeInt(1000).writeShort(classLen)
203+
.writeShort(headerLen).writeInt(contentLen);
204+
}
205+
206+
private static ByteBuf v1ResponseFrame(short classLen, short headerLen, int contentLen) {
207+
return Unpooled.buffer(RpcProtocol.getResponseHeaderLength())
208+
.writeByte(RpcProtocol.PROTOCOL_CODE).writeByte(RpcCommandType.RESPONSE)
209+
.writeShort(RpcCommandCode.RPC_RESPONSE.value()).writeByte(1).writeInt(1).writeByte(1)
210+
.writeShort(0).writeShort(classLen).writeShort(headerLen).writeInt(contentLen);
211+
}
212+
213+
private static ByteBuf v2ResponseFrame(short classLen, short headerLen, int contentLen) {
214+
return Unpooled.buffer(RpcProtocolV2.getResponseHeaderLength())
215+
.writeByte(RpcProtocolV2.PROTOCOL_CODE).writeByte(RpcProtocolV2.PROTOCOL_VERSION_1)
216+
.writeByte(RpcCommandType.RESPONSE).writeShort(RpcCommandCode.RPC_RESPONSE.value())
217+
.writeByte(1).writeInt(1).writeByte(1).writeByte(0).writeShort(0).writeShort(classLen)
218+
.writeShort(headerLen).writeInt(contentLen);
219+
}
220+
221+
private static ChannelHandlerContext context() {
222+
ChannelHandlerContext context = Mockito.mock(ChannelHandlerContext.class);
223+
Mockito.when(context.channel()).thenReturn(Mockito.mock(Channel.class));
224+
return context;
225+
}
226+
}

0 commit comments

Comments
 (0)