BitoAgent commented on code in PR #13786:
URL: https://github.com/apache/dubbo/pull/13786#discussion_r1573858716
##########
dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/h12/grpc/GrpcHttp2ServerTransportListener.java:
##########
@@ -145,39 +146,31 @@ public void onMessage(InputStream inputStream) {
private class DetermineMethodDescriptorListener implements
StreamingDecoder.FragmentListener {
- @Override
- public void onFragmentMessage(InputStream rawMessage) {}
-
@Override
public void onClose() {
getStreamingDecoder().close();
}
@Override
- public void onFragmentMessage(InputStream dataHeader, InputStream
rawMessage) {
+ public void onFragmentMessage(InputStream rawMessage) {
try {
- ByteArrayOutputStream merged =
- new ByteArrayOutputStream(dataHeader.available() +
rawMessage.available());
- StreamUtils.copy(dataHeader, merged);
- byte[] data = StreamUtils.readBytes(rawMessage);
-
RpcInvocationBuildContext context = getContext();
if (null == context.getMethodDescriptor()) {
-
context.setMethodDescriptor(DescriptorUtils.findTripleMethodDescriptor(
- context.getServiceDescriptor(),
context.getMethodName(), data));
+ byte[] data = StreamUtils.readBytes(rawMessage);
+ MethodDescriptor methodDescriptor =
DescriptorUtils.findTripleMethodDescriptor(
+ context.getServiceDescriptor(),
context.getMethodName(), data);
+ context.setMethodDescriptor(methodDescriptor);
setHttpMessageListener(GrpcHttp2ServerTransportListener.super.buildHttpMessageListener());
// replace decoder
GrpcCompositeCodec grpcCompositeCodec =
(GrpcCompositeCodec) context.getHttpMessageDecoder();
- MethodMetadata methodMetadata =
context.getMethodMetadata();
-
grpcCompositeCodec.setDecodeTypes(methodMetadata.getActualRequestTypes());
- grpcCompositeCodec.setEncodeTypes(new Class[]
{methodMetadata.getActualResponseType()});
+ grpcCompositeCodec.loadPackableMethod(methodDescriptor);
getServerChannelObserver().setResponseEncoder(grpcCompositeCodec);
+ rawMessage = new ByteArrayInputStream(data);
}
- merged.write(data);
- getHttpMessageListener().onMessage(new
ByteArrayInputStream(merged.toByteArray()));
+ getStreamingDecoder().invokeListener(rawMessage);
} catch (IOException e) {
throw new DecodeException(e);
}
Review Comment:
**Security Issue**: The implementation of onFragmentMessage does not
validate the input before processing, which could lead to security
vulnerabilities such as injection attacks or buffer overflows. <br> **Fix**:
Implement input validation for the rawMessage InputStream before processing.
Ensure that the data conforms to expected formats and sizes to prevent
injection attacks or other forms of malicious input. <br> **Code Suggestion**:
```
Implement input validation for the rawMessage InputStream before
processing. Ensure that the data conforms to expected formats and sizes to
prevent injection attacks or other forms of malicious input.
```
##########
dubbo-remoting/dubbo-remoting-http12/src/main/java/org/apache/dubbo/remoting/http12/message/LengthFieldStreamingDecoder.java:
##########
@@ -130,16 +127,12 @@ private void deliver() {
}
private void processHeader() throws IOException {
- ByteArrayOutputStream bos = new
ByteArrayOutputStream(lengthFieldOffset + lengthFieldLength);
byte[] offsetData = new byte[lengthFieldOffset];
int ignore = accumulate.read(offsetData);
- bos.write(offsetData);
processOffset(new ByteArrayInputStream(offsetData), lengthFieldOffset);
byte[] lengthBytes = new byte[lengthFieldLength];
ignore = accumulate.read(lengthBytes);
- bos.write(lengthBytes);
requiredLength = bytesToInt(lengthBytes);
- this.dataHeader = new ByteArrayInputStream(bos.toByteArray());
// Continue reading the frame body.
state = DecodeState.PAYLOAD;
Review Comment:
**Security Issue**: The method 'processHeader' lacks proper validation of
the header length, potentially leading to buffer overflow or denial of service
if an attacker sends crafted input with a negative size or size that's
excessively large. <br> **Fix**: Implement checks to ensure that the header
length (both 'lengthFieldOffset' and 'lengthFieldLength') is within expected
bounds before processing it. <br> **Code Suggestion**:
```
private void processHeader() throws IOException {
byte[] offsetData = new byte[lengthFieldOffset];
int ignore = accumulate.read(offsetData);
if (lengthFieldOffset < 0 || lengthFieldLength < 0 || lengthFieldOffset
+ lengthFieldLength > accumulate.available()) {
throw new IllegalArgumentException("Invalid header size");
}
processOffset(new ByteArrayInputStream(offsetData), lengthFieldOffset);
byte[] lengthBytes = new byte[lengthFieldLength];
ignore = accumulate.read(lengthBytes);
requiredLength = bytesToInt(lengthBytes);
}
```
--
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: notifications-unsubscr...@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@dubbo.apache.org
For additional commands, e-mail: notifications-h...@dubbo.apache.org
