Skip to content

Commit 711343e

Browse files
chickenljchickenlj
committed
fix deserialization vulnerability
1 parent 27571a0 commit 711343e

15 files changed

Lines changed: 198 additions & 40 deletions

File tree

‎dubbo-common/src/main/java/org/apache/dubbo/common/BaseServiceMetadata.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818

1919
import org.apache.dubbo.common.utils.StringUtils;
2020

21+
import static org.apache.dubbo.common.constants.CommonConstants.DEFAULT_VERSION;
22+
2123
/**
2224
* 2019-10-10
2325
*/
@@ -44,7 +46,7 @@ public static String buildServiceKey(String path, String group, String version)
4446
public static String versionFromServiceKey(String serviceKey) {
4547
int index = serviceKey.indexOf(":");
4648
if (index == -1) {
47-
return null;
49+
return DEFAULT_VERSION;
4850
}
4951
return serviceKey.substring(index + 1);
5052
}

‎dubbo-common/src/main/java/org/apache/dubbo/common/constants/CommonConstants.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -364,4 +364,6 @@ public interface CommonConstants {
364364
String SENTINEL_REDIS = "sentinel";
365365

366366
String CLUSTER_REDIS = "cluster";
367+
368+
String DEFAULT_VERSION = "0.0.0";
367369
}

‎dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/exchange/codec/ExchangeCodec.java‎

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -152,9 +152,9 @@ protected Object decodeBody(Channel channel, InputStream is, byte[] header) thro
152152
if (status == Response.OK) {
153153
Object data;
154154
if (res.isHeartbeat()) {
155-
data = decodeHeartbeatData(channel, in);
155+
data = decodeHeartbeatData(channel, in, is);
156156
} else if (res.isEvent()) {
157-
data = decodeEventData(channel, in);
157+
data = decodeEventData(channel, in, is);
158158
} else {
159159
data = decodeResponseData(channel, in, getRequestData(id));
160160
}
@@ -179,9 +179,9 @@ protected Object decodeBody(Channel channel, InputStream is, byte[] header) thro
179179
ObjectInput in = CodecSupport.deserialize(channel.getUrl(), is, proto);
180180
Object data;
181181
if (req.isHeartbeat()) {
182-
data = decodeHeartbeatData(channel, in);
182+
data = decodeHeartbeatData(channel, in, is);
183183
} else if (req.isEvent()) {
184-
data = decodeEventData(channel, in);
184+
data = decodeEventData(channel, in, is);
185185
} else {
186186
data = decodeRequestData(channel, in);
187187
}
@@ -208,7 +208,7 @@ protected Object getRequestData(long id) {
208208
}
209209

210210
protected void encodeRequest(Channel channel, ChannelBuffer buffer, Request req) throws IOException {
211-
Serialization serialization = getSerialization(channel);
211+
Serialization serialization = getSerialization(channel, req);
212212
// header.
213213
byte[] header = new byte[HEADER_LENGTH];
214214
// set magic number.
@@ -256,7 +256,7 @@ protected void encodeRequest(Channel channel, ChannelBuffer buffer, Request req)
256256
protected void encodeResponse(Channel channel, ChannelBuffer buffer, Response res) throws IOException {
257257
int savedWriteIndex = buffer.writerIndex();
258258
try {
259-
Serialization serialization = getSerialization(channel);
259+
Serialization serialization = getSerialization(channel, res);
260260
// header.
261261
byte[] header = new byte[HEADER_LENGTH];
262262
// set magic number.
@@ -347,11 +347,6 @@ protected Object decodeData(ObjectInput in) throws IOException {
347347
return decodeRequestData(in);
348348
}
349349

350-
@Deprecated
351-
protected Object decodeHeartbeatData(ObjectInput in) throws IOException {
352-
return decodeEventData(null, in);
353-
}
354-
355350
protected Object decodeRequestData(ObjectInput in) throws IOException {
356351
try {
357352
return in.readObject();
@@ -395,17 +390,21 @@ protected Object decodeData(Channel channel, ObjectInput in) throws IOException
395390
return decodeRequestData(channel, in);
396391
}
397392

398-
protected Object decodeEventData(Channel channel, ObjectInput in) throws IOException {
393+
protected Object decodeEventData(Channel channel, ObjectInput in, InputStream is) throws IOException {
399394
try {
395+
int dataLen = is.available();
396+
if (dataLen > 100) {
397+
throw new IllegalArgumentException("Event data too long, rejected by deserialization security check.");
398+
}
400399
return in.readEvent();
401400
} catch (IOException | ClassNotFoundException e) {
402401
throw new IOException(StringUtils.toString("Decode dubbo protocol event failed.", e));
403402
}
404403
}
405404

406405
@Deprecated
407-
protected Object decodeHeartbeatData(Channel channel, ObjectInput in) throws IOException {
408-
return decodeEventData(channel, in);
406+
protected Object decodeHeartbeatData(Channel channel, ObjectInput in, InputStream is) throws IOException {
407+
return decodeEventData(channel, in, is);
409408
}
410409

411410
protected Object decodeRequestData(Channel channel, ObjectInput in) throws IOException {

‎dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/transport/AbstractCodec.java‎

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,6 @@
1616
*/
1717
package org.apache.dubbo.remoting.transport;
1818

19-
import java.io.IOException;
20-
import java.net.InetSocketAddress;
21-
2219
import org.apache.dubbo.common.URL;
2320
import org.apache.dubbo.common.logger.Logger;
2421
import org.apache.dubbo.common.logger.LoggerFactory;
@@ -27,6 +24,11 @@
2724
import org.apache.dubbo.remoting.Channel;
2825
import org.apache.dubbo.remoting.Codec2;
2926
import org.apache.dubbo.remoting.Constants;
27+
import org.apache.dubbo.remoting.exchange.Request;
28+
import org.apache.dubbo.remoting.exchange.Response;
29+
30+
import java.io.IOException;
31+
import java.net.InetSocketAddress;
3032

3133
import static org.apache.dubbo.common.constants.CommonConstants.SIDE_KEY;
3234

@@ -48,18 +50,26 @@ protected static void checkPayload(Channel channel, long size) throws IOExceptio
4850
}
4951
if (payload > 0 && size > payload) {
5052
ExceedPayloadLimitException e = new ExceedPayloadLimitException(
51-
"Data length too large: " + size + ", max payload: " + payload + ", channel: " + channel);
53+
"Data length too large: " + size + ", max payload: " + payload + ", channel: " + channel);
5254
logger.error(e);
5355
throw e;
5456
}
5557
}
5658

59+
protected Serialization getSerialization(Channel channel, Request req) {
60+
return CodecSupport.getSerialization(channel.getUrl());
61+
}
62+
63+
protected Serialization getSerialization(Channel channel, Response res) {
64+
return CodecSupport.getSerialization(channel.getUrl());
65+
}
66+
5767
protected Serialization getSerialization(Channel channel) {
5868
return CodecSupport.getSerialization(channel.getUrl());
5969
}
6070

6171
protected boolean isClientSide(Channel channel) {
62-
String side = (String)channel.getAttribute(SIDE_KEY);
72+
String side = (String) channel.getAttribute(SIDE_KEY);
6373
if (CLIENT_SIDE.equals(side)) {
6474
return true;
6575
} else if (SERVER_SIDE.equals(side)) {

‎dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/transport/CodecSupport.java‎

Lines changed: 31 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -23,18 +23,19 @@
2323
import org.apache.dubbo.common.logger.LoggerFactory;
2424
import org.apache.dubbo.common.serialize.ObjectInput;
2525
import org.apache.dubbo.common.serialize.Serialization;
26+
import org.apache.dubbo.common.utils.CollectionUtils;
2627
import org.apache.dubbo.remoting.Constants;
28+
import org.apache.dubbo.rpc.model.ApplicationModel;
29+
import org.apache.dubbo.rpc.model.ProviderModel;
30+
import org.apache.dubbo.rpc.model.ServiceRepository;
2731

2832
import java.io.IOException;
2933
import java.io.InputStream;
3034
import java.util.HashMap;
35+
import java.util.List;
3136
import java.util.Map;
3237
import java.util.Set;
3338

34-
import static org.apache.dubbo.common.serialize.Constants.COMPACTED_JAVA_SERIALIZATION_ID;
35-
import static org.apache.dubbo.common.serialize.Constants.JAVA_SERIALIZATION_ID;
36-
import static org.apache.dubbo.common.serialize.Constants.NATIVE_JAVA_SERIALIZATION_ID;
37-
3839
public class CodecSupport {
3940

4041
private static final Logger logger = LoggerFactory.getLogger(CodecSupport.class);
@@ -76,20 +77,37 @@ public static Serialization getSerialization(URL url) {
7677
url.getParameter(Constants.SERIALIZATION_KEY, Constants.DEFAULT_REMOTING_SERIALIZATION));
7778
}
7879

79-
public static Serialization getSerialization(URL url, Byte id) throws IOException {
80-
Serialization serialization = getSerializationById(id);
81-
String serializationName = url.getParameter(Constants.SERIALIZATION_KEY, Constants.DEFAULT_REMOTING_SERIALIZATION);
82-
// Check if "serialization id" passed from network matches the id on this side(only take effect for JDK serialization), for security purpose.
83-
if (serialization == null
84-
|| ((id == JAVA_SERIALIZATION_ID || id == NATIVE_JAVA_SERIALIZATION_ID || id == COMPACTED_JAVA_SERIALIZATION_ID)
85-
&& !(serializationName.equals(ID_SERIALIZATIONNAME_MAP.get(id))))) {
86-
throw new IOException("Unexpected serialization id:" + id + " received from network, please check if the peer send the right id.");
80+
public static Serialization getSerialization(URL url, Byte id) {
81+
Serialization result = getSerializationById(id);
82+
if (result == null) {
83+
result = getSerialization(url);
8784
}
88-
return serialization;
85+
return result;
8986
}
9087

9188
public static ObjectInput deserialize(URL url, InputStream is, byte proto) throws IOException {
9289
Serialization s = getSerialization(url, proto);
9390
return s.deserialize(url, is);
9491
}
92+
93+
public static void checkSerialization(String path, String version, Byte id) throws IOException {
94+
ServiceRepository repository = ApplicationModel.getServiceRepository();
95+
ProviderModel providerModel = repository.lookupExportedServiceWithoutGroup(path + ":" + version);
96+
if (providerModel == null) {
97+
if (logger.isWarnEnabled()) {
98+
logger.warn("Serialization security check is enabled but cannot work as expected because " +
99+
"there's no matched provider model for path " + path + ", version " + version);
100+
}
101+
} else {
102+
List<URL> urls = providerModel.getServiceConfig().getExportedUrls();
103+
if (CollectionUtils.isNotEmpty(urls)) {
104+
URL url = urls.get(0);
105+
String serializationName = url.getParameter(org.apache.dubbo.remoting.Constants.SERIALIZATION_KEY, Constants.DEFAULT_REMOTING_SERIALIZATION);
106+
Byte localId = SERIALIZATIONNAME_ID_MAP.get(serializationName);
107+
if (localId != null && !localId.equals(id)) {
108+
throw new IOException("Unexpected serialization id:" + id + " received from network, please check if the peer send the right id.");
109+
}
110+
}
111+
}
112+
}
95113
}

‎dubbo-remoting/dubbo-remoting-api/src/test/java/org/apache/dubbo/remoting/codec/ExchangeCodecTest.java‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,22 @@ private byte[] getRequestBytes(Object obj, byte[] header) throws IOException {
9696
return request;
9797
}
9898

99+
private byte[] getReadonlyEventRequestBytes(Object obj, byte[] header) throws IOException {
100+
// encode request data.
101+
UnsafeByteArrayOutputStream bos = new UnsafeByteArrayOutputStream(1024);
102+
ObjectOutput out = serialization.serialize(url, bos);
103+
out.writeObject(obj);
104+
105+
out.flushBuffer();
106+
bos.flush();
107+
bos.close();
108+
byte[] data = bos.toByteArray();
109+
// byte[] len = Bytes.int2bytes(data.length);
110+
System.arraycopy(data, 0, header, 12, data.length);
111+
byte[] request = join(header, data);
112+
return request;
113+
}
114+
99115
private byte[] assemblyDataProtocol(byte[] header) {
100116
Person request = new Person();
101117
byte[] newbuf = join(header, objectToByte(request));

‎dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/AppResponse.java‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@
2626
import java.util.function.BiConsumer;
2727
import java.util.function.Function;
2828

29+
import static org.apache.dubbo.remoting.Constants.SERIALIZATION_KEY;
30+
2931
/**
3032
* {@link AsyncRpcResult} is introduced in 3.0.0 to replace RpcResult, and RpcResult is replaced with {@link AppResponse}:
3133
* <ul>
@@ -55,9 +57,15 @@ public class AppResponse implements Result {
5557

5658
private Map<String, Object> attachments = new HashMap<>();
5759

60+
private Map<String, Object> attributes = new HashMap<>();
61+
5862
public AppResponse() {
5963
}
6064

65+
public AppResponse(Invocation invocation) {
66+
this.attributes.put(SERIALIZATION_KEY, invocation);
67+
}
68+
6169
public AppResponse(Object result) {
6270
this.result = result;
6371
}
@@ -212,6 +220,14 @@ public void setObjectAttachment(String key, Object value) {
212220
attachments.put(key, value);
213221
}
214222

223+
public Object getAttribute(String key) {
224+
return attributes.get(key);
225+
}
226+
227+
public void setAttribute(String key, Object value) {
228+
attributes.put(key, value);
229+
}
230+
215231
@Override
216232
public Result whenCompleteWithContext(BiConsumer<Result, Throwable> fn) {
217233
throw new UnsupportedOperationException("AppResponse represents an concrete business response, there will be no status changes, you should get internal values directly.");

‎dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/AsyncRpcResult.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -319,7 +319,7 @@ public static AsyncRpcResult newDefaultAsyncResult(Throwable t, Invocation invoc
319319

320320
public static AsyncRpcResult newDefaultAsyncResult(Object value, Throwable t, Invocation invocation) {
321321
CompletableFuture<AppResponse> future = new CompletableFuture<>();
322-
AppResponse result = new AppResponse();
322+
AppResponse result = new AppResponse(invocation);
323323
if (t != null) {
324324
result.setException(t);
325325
} else {

‎dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/Constants.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,4 +90,8 @@ public interface Constants {
9090

9191
String CONSUMER_MODEL = "consumerModel";
9292
String METHOD_MODEL = "methodModel";
93+
94+
String SERIALIZATION_SECURITY_CHECK_KEY = "serialization.security.check";
95+
String INVOCATION_KEY = "invocation";
96+
String SERIALIZATION_ID_KEY = "serialization_id";
9397
}

‎dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/proxy/AbstractProxyInvoker.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,9 +82,9 @@ public void destroy() {
8282
public Result invoke(Invocation invocation) throws RpcException {
8383
try {
8484
Object value = doInvoke(proxy, invocation.getMethodName(), invocation.getParameterTypes(), invocation.getArguments());
85-
CompletableFuture<Object> future = wrapWithFuture(value);
85+
CompletableFuture<Object> future = wrapWithFuture(value);
8686
CompletableFuture<AppResponse> appResponseFuture = future.handle((obj, t) -> {
87-
AppResponse result = new AppResponse();
87+
AppResponse result = new AppResponse(invocation);
8888
if (t != null) {
8989
if (t instanceof CompletionException) {
9090
result.setException(t.getCause());

0 commit comments

Comments
 (0)