Skip to content

Commit 76ff74e

Browse files
authored
fix: Deserialization checks valid class types for HttpTransportFactory (#1882)
* fix: Deserialization checks valid class types for HttpTransportFactory * chore: Add headers for newly added files * chore: Fix sonatype complaints * chore: Address PR comments
1 parent f9ae88c commit 76ff74e

File tree

4 files changed

+547
-5
lines changed

4 files changed

+547
-5
lines changed

appengine/java/com/google/auth/appengine/AppEngineCredentials.java

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,8 @@
4343
import com.google.errorprone.annotations.CanIgnoreReturnValue;
4444
import java.io.IOException;
4545
import java.io.ObjectInputStream;
46+
import java.lang.reflect.Constructor;
47+
import java.lang.reflect.InvocationTargetException;
4648
import java.util.Collection;
4749
import java.util.Date;
4850
import java.util.Objects;
@@ -132,7 +134,31 @@ public boolean equals(Object obj) {
132134

133135
private void readObject(ObjectInputStream input) throws IOException, ClassNotFoundException {
134136
input.defaultReadObject();
135-
appIdentityService = newInstance(appIdentityServiceClassName);
137+
try {
138+
// Load the class without initializing it (second argument: false) to prevent
139+
// static initializers from running (preventing gadget chain attacks). Use the class loader
140+
// of HttpTransportFactory to ensure the class is loaded from the same context as the library
141+
// to try to prevent any class loading manipulation.
142+
Class<?> clazz =
143+
Class.forName(
144+
appIdentityServiceClassName, false, AppIdentityService.class.getClassLoader());
145+
146+
// Check that the class is an instance of `AppIdentityService` to prevent loading of
147+
// arbitrary classes.
148+
if (!AppIdentityService.class.isAssignableFrom(clazz)) {
149+
throw new IOException(
150+
String.format(
151+
"The class, %s, is not assignable from %s.",
152+
appIdentityServiceClassName, AppIdentityService.class.getName()));
153+
}
154+
Constructor<?> constructor = clazz.getConstructor();
155+
appIdentityService = (AppIdentityService) constructor.newInstance();
156+
} catch (InstantiationException
157+
| IllegalAccessException
158+
| NoSuchMethodException
159+
| InvocationTargetException e) {
160+
throw new IOException(e);
161+
}
136162
}
137163

138164
public static Builder newBuilder() {
Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
/*
2+
* Copyright 2026, Google Inc. All rights reserved.
3+
*
4+
* Redistribution and use in source and binary forms, with or without
5+
* modification, are permitted provided that the following conditions are
6+
* met:
7+
*
8+
* * Redistributions of source code must retain the above copyright
9+
* notice, this list of conditions and the following disclaimer.
10+
* * Redistributions in binary form must reproduce the above
11+
* copyright notice, this list of conditions and the following disclaimer
12+
* in the documentation and/or other materials provided with the
13+
* distribution.
14+
*
15+
* * Neither the name of Google Inc. nor the names of its
16+
* contributors may be used to endorse or promote products derived from
17+
* this software without specific prior written permission.
18+
*
19+
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
20+
* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
21+
* LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
22+
* A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
23+
* OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
24+
* SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
25+
* LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
26+
* DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
27+
* THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
28+
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
29+
* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
30+
*/
31+
32+
package com.google.auth.appengine;
33+
34+
import static org.junit.jupiter.api.Assertions.assertEquals;
35+
import static org.junit.jupiter.api.Assertions.assertNotNull;
36+
import static org.junit.jupiter.api.Assertions.assertThrows;
37+
38+
import java.io.ByteArrayInputStream;
39+
import java.io.ByteArrayOutputStream;
40+
import java.io.IOException;
41+
import java.io.ObjectInputStream;
42+
import java.io.ObjectOutputStream;
43+
import java.lang.reflect.Field;
44+
import java.util.Collections;
45+
import org.junit.jupiter.api.Test;
46+
47+
class AppEngineDeserializationSecurityTest {
48+
49+
/** A class that does not implement HttpTransportFactory. */
50+
static class ArbitraryClass {}
51+
52+
@Test
53+
void testArbitraryClassInstantiationPrevented() throws Exception {
54+
AppEngineCredentials credentials =
55+
AppEngineCredentials.newBuilder().setScopes(Collections.singleton("scope")).build();
56+
57+
// Use reflection to set appIdentityServiceClassName to ArbitraryClass
58+
// as the setter must be of AppIdentityService
59+
Field classNameField =
60+
AppEngineCredentials.class.getDeclaredField("appIdentityServiceClassName");
61+
classNameField.setAccessible(true);
62+
classNameField.set(credentials, ArbitraryClass.class.getName());
63+
64+
ByteArrayOutputStream bos = new ByteArrayOutputStream();
65+
ObjectOutputStream oos = new ObjectOutputStream(bos);
66+
oos.writeObject(credentials);
67+
68+
ByteArrayInputStream bis = new ByteArrayInputStream(bos.toByteArray());
69+
ObjectInputStream ois = new ObjectInputStream(bis);
70+
71+
assertThrows(IOException.class, ois::readObject);
72+
73+
bos.close();
74+
oos.close();
75+
bis.close();
76+
ois.close();
77+
}
78+
79+
@Test
80+
void testValidServiceDeserialization() throws Exception {
81+
MockAppIdentityService mockService = new MockAppIdentityService();
82+
AppEngineCredentials credentials =
83+
AppEngineCredentials.newBuilder()
84+
.setScopes(Collections.singleton("scope"))
85+
.setAppIdentityService(mockService)
86+
.build();
87+
88+
ByteArrayOutputStream bos = new ByteArrayOutputStream();
89+
ObjectOutputStream oos = new ObjectOutputStream(bos);
90+
oos.writeObject(credentials);
91+
oos.close();
92+
93+
ByteArrayInputStream bis = new ByteArrayInputStream(bos.toByteArray());
94+
ObjectInputStream ois = new ObjectInputStream(bis);
95+
96+
AppEngineCredentials deserialized = (AppEngineCredentials) ois.readObject();
97+
98+
assertNotNull(deserialized);
99+
Field serviceField = AppEngineCredentials.class.getDeclaredField("appIdentityService");
100+
serviceField.setAccessible(true);
101+
Object service = serviceField.get(deserialized);
102+
assertEquals(MockAppIdentityService.class, service.getClass());
103+
104+
bos.close();
105+
oos.close();
106+
bis.close();
107+
ois.close();
108+
}
109+
110+
@Test
111+
void testNonExistentClassDeserialization() throws Exception {
112+
AppEngineCredentials credentials =
113+
AppEngineCredentials.newBuilder().setScopes(Collections.singleton("scope")).build();
114+
115+
// 2. Use reflection to set appIdentityServiceClassName to non-existent class
116+
Field classNameField =
117+
AppEngineCredentials.class.getDeclaredField("appIdentityServiceClassName");
118+
classNameField.setAccessible(true);
119+
classNameField.set(credentials, "com.google.nonexistent.Class");
120+
121+
ByteArrayOutputStream bos = new ByteArrayOutputStream();
122+
ObjectOutputStream oos = new ObjectOutputStream(bos);
123+
oos.writeObject(credentials);
124+
oos.close();
125+
126+
ByteArrayInputStream bis = new ByteArrayInputStream(bos.toByteArray());
127+
ObjectInputStream ois = new ObjectInputStream(bis);
128+
129+
assertThrows(ClassNotFoundException.class, ois::readObject);
130+
131+
bos.close();
132+
oos.close();
133+
bis.close();
134+
ois.close();
135+
}
136+
}

oauth2_http/java/com/google/auth/oauth2/OAuth2Credentials.java

Lines changed: 57 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import com.google.auth.Credentials;
3636
import com.google.auth.RequestMetadataCallback;
3737
import com.google.auth.http.AuthHttpConstants;
38+
import com.google.auth.http.HttpTransportFactory;
3839
import com.google.common.annotations.VisibleForTesting;
3940
import com.google.common.base.MoreObjects;
4041
import com.google.common.base.Preconditions;
@@ -51,6 +52,8 @@
5152
import java.io.IOException;
5253
import java.io.ObjectInputStream;
5354
import java.io.Serializable;
55+
import java.lang.reflect.Constructor;
56+
import java.lang.reflect.InvocationTargetException;
5457
import java.net.URI;
5558
import java.time.Duration;
5659
import java.util.ArrayList;
@@ -475,11 +478,61 @@ private void readObject(ObjectInputStream input) throws IOException, ClassNotFou
475478
refreshTask = null;
476479
}
477480

478-
@SuppressWarnings("unchecked")
479-
protected static <T> T newInstance(String className) throws IOException, ClassNotFoundException {
481+
/**
482+
* Best-effort safe mechanism to attempt to instantiate an {@link HttpTransportFactory} from a
483+
* class name.
484+
*
485+
* <p>This method attempts to avoid Arbitrary Code Execution (ACE) vulnerabilities by:
486+
*
487+
* <ol>
488+
* <li>Checking if the class name matches the default or ServiceLoader-provided factory, and
489+
* returning that instance if so.
490+
* <li>If not, loading the class using reflection without running static initializers.
491+
* <li>Verifying that the loaded class is assignable to {@link HttpTransportFactory}.
492+
* <li>Only after verification, instantiating the class using its default constructor.
493+
* </ol>
494+
*
495+
* @param className The fully qualified name of the class to instantiate.
496+
* @return An instance of {@link HttpTransportFactory}.
497+
* @throws IOException If the class cannot be loaded, is the wrong type, or cannot be
498+
* instantiated.
499+
* @throws ClassNotFoundException If the class cannot be found.
500+
*/
501+
protected static HttpTransportFactory newInstance(String className)
502+
throws IOException, ClassNotFoundException {
503+
// Check if the requested class matches the default transport or ServiceLoader-provided
504+
// transport. This avoids unsafe reflection for the most common use cases. This check runs first
505+
// to replicate the logic in each Credential's constructor.
506+
HttpTransportFactory currentFactory =
507+
getFromServiceLoader(HttpTransportFactory.class, OAuth2Utils.HTTP_TRANSPORT_FACTORY);
508+
// If this doesn't match, then it may be a custom implementation of HttpTransportFactory
509+
if (className.equals(currentFactory.getClass().getName())) {
510+
return currentFactory;
511+
}
512+
513+
// Fallback to reflection to initialize the transport if the requested class is not from
514+
// ServiceLoader or the default value. This handles cases where a custom factory was used.
480515
try {
481-
return (T) Class.forName(className).newInstance();
482-
} catch (InstantiationException | IllegalAccessException e) {
516+
// Load the class without initializing it (second argument: false) to prevent
517+
// static initializers from running (preventing gadget chain attacks). Use the class loader
518+
// of HttpTransportFactory to ensure the class is loaded from the same context as the library
519+
// to try to prevent any class loading manipulation.
520+
Class<?> clazz = Class.forName(className, false, HttpTransportFactory.class.getClassLoader());
521+
522+
// Check that the class is an instance of `HttpTransportFactory` to prevent loading of
523+
// arbitrary classes.
524+
if (!HttpTransportFactory.class.isAssignableFrom(clazz)) {
525+
throw new IOException(
526+
String.format(
527+
"Class: %s, is not assignable from %s",
528+
className, HttpTransportFactory.class.getName()));
529+
}
530+
Constructor<?> constructor = clazz.getDeclaredConstructor();
531+
return (HttpTransportFactory) constructor.newInstance();
532+
} catch (InstantiationException
533+
| IllegalAccessException
534+
| NoSuchMethodException
535+
| InvocationTargetException e) {
483536
throw new IOException(e);
484537
}
485538
}

0 commit comments

Comments
 (0)