Skip to content
Merged
20 changes: 20 additions & 0 deletions java/core/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,7 @@ junit_tests(
"src/test/java/com/google/protobuf/TestUtil.java",
"src/test/java/com/google/protobuf/TestUtilLite.java",
"src/test/java/com/google/protobuf/GeneratedMessagePre22WarningDisabledTest.java",
"src/test/java/com/google/protobuf/GeneratedMessagePre22ErrorTest.java",
],
),
data = ["//src/google/protobuf:testdata"],
Expand Down Expand Up @@ -473,6 +474,7 @@ LITE_TEST_EXCLUSIONS = [
"src/test/java/com/google/protobuf/FieldPresenceTest.java",
"src/test/java/com/google/protobuf/ForceFieldBuildersPreRun.java",
"src/test/java/com/google/protobuf/GeneratedMessagePre22WarningDisabledTest.java",
"src/test/java/com/google/protobuf/GeneratedMessagePre22ErrorTest.java",
"src/test/java/com/google/protobuf/GeneratedMessageTest.java",
"src/test/java/com/google/protobuf/LazyFieldTest.java",
"src/test/java/com/google/protobuf/LazyStringEndToEndTest.java",
Expand Down Expand Up @@ -545,6 +547,24 @@ java_test(
],
)

java_test(
name = "GeneratedMessagePre22ErrorTest",
size = "small",
srcs = [
"src/test/java/com/google/protobuf/GeneratedMessagePre22ErrorTest.java",
],
jvm_flags = ["-Dcom.google.protobuf.error_on_unsafe_pre22_gencode"],
deps = [
":core",
":generic_test_protos_java_proto",
":java_test_protos_java_proto",
":lite_test_protos_java_proto",
":test_util",
"@maven//:com_google_truth_truth",
"@maven//:junit_junit",
],
)

pkg_files(
name = "dist_files",
srcs = glob([
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import java.util.List;
import java.util.Map;
import java.util.TreeMap;
import java.util.logging.Logger;

/**
* All generated protocol message classes extend this class. This class implements most of the
Expand All @@ -35,6 +36,7 @@
*/
public abstract class GeneratedMessage extends AbstractMessage implements Serializable {
private static final long serialVersionUID = 1L;
private static final Logger logger = Logger.getLogger(GeneratedMessage.class.getName());

/**
* For testing. Allows a test to disable the optimization that avoids using field builders for
Expand Down Expand Up @@ -310,22 +312,33 @@ public int getSerializedSize() {
return memoizedSize;
}

static final String PRE22_GENCODE_ACKNOWLEGE_PROPERTY =
static final String PRE22_GENCODE_SILENCE_PROPERTY =
"com.google.protobuf.use_unsafe_pre22_gencode";
static final String PRE22_GENCODE_ERROR_PROPERTY =
"com.google.protobuf.error_on_unsafe_pre22_gencode";

static final String PRE22_GENCODE_VULNERABILITY_MESSAGE =
"As of 2022/09/29 (release 21.7) makeExtensionsImmutable should not be called from protobuf"
+ " gencode. If you are seeing this message, your gencode is vulnerable to a denial of"
+ " service attack. You should regenerate your code using protobuf 25.6 or later. Use the"
+ " latest version that meets your needs. However, if you understand the risks and wish"
+ " to continue with vulnerable gencode, you can set the system property"
+ " `-Dcom.google.protobuf.use_unsafe_pre22_gencode` on the command line. See security"
+ " vulnerability:"
+ " `-Dcom.google.protobuf.use_unsafe_pre22_gencode` on the command line to silence this"
+ " warning. You also can set"
+ " `-Dcom.google.protobuf.error_on_unsafe_pre22_gencode` to throw an error instead. See"
+ " security vulnerability:"
+ " https://github.com/protocolbuffers/protobuf/security/advisories/GHSA-h4h5-3hr4-j3g2";

static void warnPre22Gencode() {
if (System.getProperty(PRE22_GENCODE_ACKNOWLEGE_PROPERTY) == null) {
throw new UnsupportedOperationException(PRE22_GENCODE_VULNERABILITY_MESSAGE);
if (System.getProperty(PRE22_GENCODE_SILENCE_PROPERTY) != null) {
return;
}
UnsupportedOperationException exception =
new UnsupportedOperationException(PRE22_GENCODE_VULNERABILITY_MESSAGE);
if (System.getProperty(PRE22_GENCODE_ERROR_PROPERTY) != null) {
throw exception;
}
logger.warning(exception.toString());
}

/** Used by parsing constructors in generated classes. */
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package com.google.protobuf;

import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.assertThrows;

import protobuf_unittest.UnittestProto.TestAllExtensions;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;

@RunWith(JUnit4.class)
public class GeneratedMessagePre22ErrorTest {
@Test
public void generatedMessage_makeExtensionsImmutableShouldError() {
GeneratedMessageV3 msg =
new GeneratedMessageV3() {
@Override
protected FieldAccessorTable internalGetFieldAccessorTable() {
return null;
}

@Override
protected Message.Builder newBuilderForType(BuilderParent parent) {
return null;
}

@Override
public Message.Builder newBuilderForType() {
return null;
}

@Override
public Message.Builder toBuilder() {
return null;
}

@Override
public Message getDefaultInstanceForType() {
return null;
}
};
Throwable e = assertThrows(UnsupportedOperationException.class, () -> msg.makeExtensionsImmutable());
assertThat(e).hasMessageThat().contains(GeneratedMessage.PRE22_GENCODE_VULNERABILITY_MESSAGE);
}

@Test
public void extendableMessage_makeExtensionsImmutableShouldError() {
GeneratedMessageV3.ExtendableMessage<TestAllExtensions> msg =
TestAllExtensions.newBuilder().build();
Throwable e = assertThrows(UnsupportedOperationException.class, () -> msg.makeExtensionsImmutable());
assertThat(e).hasMessageThat().contains(GeneratedMessage.PRE22_GENCODE_VULNERABILITY_MESSAGE);
}
}
Original file line number Diff line number Diff line change
@@ -1,14 +1,27 @@
package com.google.protobuf;

import static com.google.common.truth.Truth.assertThat;

import protobuf_unittest.UnittestProto.TestAllExtensions;
import java.util.logging.Level;
import java.util.logging.Logger;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;

@RunWith(JUnit4.class)
public class GeneratedMessagePre22WarningDisabledTest {
private TestUtil.TestLogHandler setupLogger() {
TestUtil.TestLogHandler logHandler = new TestUtil.TestLogHandler();
Logger logger = Logger.getLogger(GeneratedMessage.class.getName());
logger.addHandler(logHandler);
logHandler.setLevel(Level.ALL);
return logHandler;
}

@Test
public void generatedMessage_makeExtensionsImmutableShouldNotThrow() {
public void generatedMessage_makeExtensionsImmutableShouldNotLog() {
TestUtil.TestLogHandler logHandler = setupLogger();
GeneratedMessageV3 msg =
new GeneratedMessageV3() {
@Override
Expand Down Expand Up @@ -37,13 +50,16 @@ public Message getDefaultInstanceForType() {
}
};
msg.makeExtensionsImmutable();
assertThat(logHandler.getStoredLogRecords()).isEmpty();
}

@Test
public void extendableMessage_makeExtensionsImmutableShouldNotThrow() {
public void extendableMessage_makeExtensionsImmutableShouldNotLog() {
TestUtil.TestLogHandler logHandler = setupLogger();
GeneratedMessageV3.ExtendableMessage<TestAllExtensions> msg =
TestAllExtensions.newBuilder().build();
msg.makeExtensionsImmutable();
assertThat(logHandler.getStoredLogRecords()).isEmpty();
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@
import java.util.Collections;
import java.util.Iterator;
import java.util.List;
import java.util.logging.Level;
import java.util.logging.LogRecord;
import java.util.logging.Logger;
import org.junit.After;
import org.junit.Test;
import org.junit.runner.RunWith;
Expand Down Expand Up @@ -1999,9 +2002,19 @@ public void extendableBuilder_mergeFrom_repeatedField_doesNotInvalidateExistingB
assertThat(builder.getRepeatedField(REPEATED_NESTED_MESSAGE_EXTENSION, 0))
.isEqualTo(NestedMessage.newBuilder().setBb(100).build());
}

private TestUtil.TestLogHandler setupLogger() {
TestUtil.TestLogHandler logHandler = new TestUtil.TestLogHandler();
Logger logger = Logger.getLogger(GeneratedMessage.class.getName());
logger.addHandler(logHandler);
logHandler.setLevel(Level.ALL);
return logHandler;
}


@Test
public void generatedMessage_makeExtensionsImmutableShouldThrow() {
public void generatedMessage_makeExtensionsImmutableShouldLog() {
TestUtil.TestLogHandler logHandler = setupLogger();
GeneratedMessageV3 msg =
new GeneratedMessageV3() {
@Override
Expand Down Expand Up @@ -2029,27 +2042,24 @@ public Message getDefaultInstanceForType() {
return null;
}
};
try {
msg.makeExtensionsImmutable();
assertWithMessage("Expected UnsupportedOperationException").fail();
} catch (UnsupportedOperationException e) {
// Expected
assertThat(e).hasMessageThat().contains(GeneratedMessage.PRE22_GENCODE_VULNERABILITY_MESSAGE);
assertThat(e).hasMessageThat().contains(GeneratedMessage.PRE22_GENCODE_ACKNOWLEGE_PROPERTY);
}
msg.makeExtensionsImmutable();
List<LogRecord> logs = logHandler.getStoredLogRecords();
assertThat(logs).hasSize(1);
String message = logs.get(0).getMessage();
assertThat(message).contains(GeneratedMessage.PRE22_GENCODE_VULNERABILITY_MESSAGE);
assertThat(message).contains(GeneratedMessage.PRE22_GENCODE_SILENCE_PROPERTY);
}

@Test
public void extendableMessage_makeExtensionsImmutableShouldThrow() {
TestUtil.TestLogHandler logHandler = setupLogger();
GeneratedMessageV3.ExtendableMessage<TestAllExtensions> msg =
TestAllExtensions.getDefaultInstance();
try {
msg.makeExtensionsImmutable();
assertWithMessage("Expected UnsupportedOperationException").fail();
} catch (UnsupportedOperationException e) {
// Expected
assertThat(e).hasMessageThat().contains(GeneratedMessage.PRE22_GENCODE_VULNERABILITY_MESSAGE);
assertThat(e).hasMessageThat().contains(GeneratedMessage.PRE22_GENCODE_ACKNOWLEGE_PROPERTY);
}
msg.makeExtensionsImmutable();
List<LogRecord> logs = logHandler.getStoredLogRecords();
assertThat(logs).hasSize(1);
String message = logs.get(0).getMessage();
assertThat(message).contains(GeneratedMessage.PRE22_GENCODE_VULNERABILITY_MESSAGE);
assertThat(message).contains(GeneratedMessage.PRE22_GENCODE_SILENCE_PROPERTY);
}
}
Loading