Signal/Signal-Server
1
0
Fork 0
mirror of https://github.com/signalapp/Signal-Server synced 2026-02-15 11:15:44 +00:00
Code Issues Packages Projects Releases Wiki Activity

Add nested message validation

Browse Source
This commit is contained in:
ravi-signal
2026-01-05 17:23:11 -05:00
committed by GitHub
parent 9c4047a90b
commit eaabbd5188
4 changed files with 70 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

17
service/src/main/java/org/whispersystems/textsecuregcm/grpc/ValidatingInterceptor.java
View File

@@ -15,6 +15,7 @@ import io.grpc.ServerCall;
import io.grpc.ServerCallHandler;
import io.grpc.ServerInterceptor;
import io.grpc.StatusException;
import java.util.List;
import java.util.Map;
import org.whispersystems.textsecuregcm.grpc.validators.E164FieldValidator;
import org.whispersystems.textsecuregcm.grpc.validators.EnumSpecifiedFieldValidator;
@@ -81,12 +82,28 @@ public class ValidatingInterceptor implements ServerInterceptor {
for (final Map.Entry<Descriptors.FieldDescriptor, Object> entry: fd.getOptions().getAllFields().entrySet()) {
final Descriptors.FieldDescriptor extensionFieldDescriptor = entry.getKey();
final String extensionName = extensionFieldDescriptor.getFullName();
// first validate the field
final FieldValidator validator = fieldValidators.get(extensionName);
// not all extensions are validators, so `validator` value here could legitimately be `null`
if (validator != null) {
validator.validate(entry.getValue(), fd, msg);
}
}
// Recursively validate the field's value(s) if it is a message or a repeated field
// gRPC's proto deserialization limits nesting to 100 so this has bounded stack usage
if (fd.isRepeated() && msg.getField(fd) instanceof List list) {
// Checking for repeated fields also handles maps, because maps are syntax sugar for repeated MapEntries
// which themselves are Messages that will be recursively descended.
for (final Object o : list) {
validateMessage(o);
}
} else if (fd.hasPresence() && msg.hasField(fd)) {
// If the field has presence information and is present, recursively validate it. Not all fields have
// presence, but we only validate Message type fields anyway, which always have explicit presence.
validateMessage(msg.getField(fd));
}
}
} catch (final StatusException e) {
throw e;

2
service/src/main/proto/org/signal/chat/backups.proto
View File

@@ -408,7 +408,7 @@ message CopyMediaItem {
/**
* The attachment cdn of the object to copy into the backup
*/
int32 source_attachment_cdn = 1 [(require.present) = true];
int32 source_attachment_cdn = 1 [(require.range).min = 1, (require.range).max = 3];
/**
* The attachment key of the object to copy into the backup

38
service/src/test/java/org/whispersystems/textsecuregcm/grpc/ValidatingInterceptorTest.java
View File

@@ -32,9 +32,11 @@ import org.junit.jupiter.params.provider.MethodSource;
import org.junit.jupiter.params.provider.ValueSource;
import org.signal.chat.require.Auth;
import org.signal.chat.rpc.Color;
import org.signal.chat.rpc.NestedMessage;
import org.signal.chat.rpc.ReactorAnonymousServiceGrpc;
import org.signal.chat.rpc.ReactorAuthServiceGrpc;
import org.signal.chat.rpc.ReactorValidationTestServiceGrpc;
import org.signal.chat.rpc.RecursiveMessage;
import org.signal.chat.rpc.ValidationTestServiceGrpc;
import org.signal.chat.rpc.ValidationsRequest;
import org.signal.chat.rpc.ValidationsResponse;
@@ -374,6 +376,37 @@ public class ValidatingInterceptorTest {
stub.validationsEndpoint(builderWithValidDefaults().build());
}
@Test
public void testFailedValidationOnNestedMessage() {
assertStatusException(Status.INVALID_ARGUMENT, () ->
stub.validationsEndpoint(builderWithValidDefaults().setNested(NestedMessage.newBuilder().setI32(101)).build()));
assertStatusException(Status.INVALID_ARGUMENT, () ->
stub.validationsEndpoint(builderWithValidDefaults()
.clearRepeatedNested()
.addRepeatedNested(NestedMessage.newBuilder().setI32(101)).build()));
assertStatusException(Status.INVALID_ARGUMENT, () ->
stub.validationsEndpoint(builderWithValidDefaults()
.clearMapNested()
.putMapNested("foo", NestedMessage.newBuilder().setI32(101).build()).build()));
}
@Test
public void deeplyNestedValidation() throws Exception {
// The default proto nesting limit (what we use in gRPC) is 100. Make sure we can handle twice that amount of nesting
int numNestedMessages = 100 * 2;
RecursiveMessage.Builder curr = RecursiveMessage.newBuilder().setI32(101);
for (int i = 0; i < numNestedMessages; i++) {
final RecursiveMessage.Builder pred = RecursiveMessage.newBuilder();
pred.setNext(curr);
curr = pred;
}
final RecursiveMessage recursiveMessage = curr.build();
assertStatusException(Status.INVALID_ARGUMENT, () ->
stub.validationsEndpoint(builderWithValidDefaults().setRecursiveMessage(recursiveMessage).build()));
}
@Nonnull
private static ValidationsRequest.Builder builderWithValidDefaults() {
return ValidationsRequest.newBuilder()
@@ -398,7 +431,10 @@ public class ValidatingInterceptorTest {
.setRangeSizeBytes(ByteString.copyFrom(new byte[3]))
.addAllFixedSizeList(List.of("a", "b", "c", "d", "e"))
.addAllRangeSizeList(List.of("a", "b", "c", "d", "e"))
.setI32Range(15);
.setI32Range(15)
.setNested(NestedMessage.getDefaultInstance())
.addRepeatedNested(NestedMessage.getDefaultInstance())
.putMapNested("test", NestedMessage.getDefaultInstance());
}
private static void assertStatusException(final Status expected, final Executable serviceCall) {

15
service/src/test/proto/validation_test.proto
View File

@@ -66,6 +66,21 @@ message ValidationsRequest {
message RequirePresentMessage {}
RequirePresentMessage presentMessage = 25 [(require.present) = true];
optional RequirePresentMessage optionalPresentMessage = 26 [(require.present) = true];
NestedMessage nested = 27;
repeated NestedMessage repeatedNested = 28;
map<string, NestedMessage> mapNested = 29;
RecursiveMessage recursiveMessage = 30;
}
message NestedMessage {
int32 i32 = 1 [(require.range).max = 100];
}
message RecursiveMessage {
RecursiveMessage next = 1;
int32 i32 = 2 [(require.range).max = 100];
}
message MessageWithInvalidRangeConstraint {