BitoAgent commented on code in PR #13786:
URL: https://github.com/apache/dubbo/pull/13786#discussion_r1573616715
##########
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 removal of ByteArrayOutputStream usage and direct
passing of byte arrays without validation can lead to security vulnerabilities
related to improper input validation. <br> **Fix**: Validate the byte arrays
for expected format and size before processing to mitigate potential buffer
overflow or data corruption vulnerabilities. <br> **Code Suggestion**:
```
byte[] offsetData = new byte[lengthFieldOffset];
int ignore = accumulate.read(offsetData);
+ if (!validateByteArray(offsetData)) {
+ throw new InvalidDataException("Invalid byte array format");
+ }
processOffset(new ByteArrayInputStream(offsetData), lengthFieldOffset);
```
##########
dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/h12/grpc/GrpcRequestHandlerMapping.java:
##########
@@ -42,9 +43,16 @@ protected boolean supportContentType(String contentType) {
@Override
protected void determineHttpMessageCodec(RequestHandler handler, URL url,
HttpRequest request) {
- HttpMessageCodec codec = CODEC_FACTORY.createCodec(url,
getFrameworkModel(), request.contentType());
- handler.setHttpMessageDecoder(codec);
- handler.setHttpMessageEncoder(codec);
+ GrpcCompositeCodec grpcCompositeCodec =
+ (GrpcCompositeCodec) CODEC_FACTORY.createCodec(url,
getFrameworkModel(), request.contentType());
+ MethodDescriptor methodDescriptor =
DescriptorUtils.findMethodDescriptor(
+ handler.getServiceDescriptor(), handler.getMethodName(),
handler.isHasStub());
+ if (methodDescriptor != null) {
+ handler.setMethodDescriptor(methodDescriptor);
+ grpcCompositeCodec.loadPackableMethod(methodDescriptor);
Review Comment:
**Security Issue**: The method 'DescriptorUtils.findMethodDescriptor' is
used without input validation, potentially allowing for injection attacks. <br>
**Fix**: Validate or sanitize 'handler.getMethodName()' and
'handler.isHasStub()' before using them in
'DescriptorUtils.findMethodDescriptor'. <br> **Code Suggestion**:
```
+ String methodName = sanitize(handler.getMethodName());
+ boolean hasStub = handler.isHasStub(); // Ensure 'hasStub' is
validated if necessary
+ MethodDescriptor methodDescriptor =
DescriptorUtils.findMethodDescriptor(
+ handler.getServiceDescriptor(), methodName, hasStub);
+ if (methodDescriptor != null) {
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
