Skip to content

Commit 3e13562

Browse files
jknackjknack
committed
security: pac4j can cause deserialization of untrusted data
- Prevent setting encoded values outside pac4j internals - Link to security advisory: GHSA-7c5v-895v-w4q5
1 parent 3be4344 commit 3e13562

File tree

10 files changed

+755
-342
lines changed

10 files changed

+755
-342
lines changed

‎modules/jooby-bom/pom.xml

+303-302
Large diffs are not rendered by default.

‎modules/jooby-pac4j/src/main/java/io/jooby/internal/pac4j/Pac4jSession.java

+126
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
/*
2+
* Jooby https://jooby.io
3+
* Apache License Version 2.0 https://jooby.io/LICENSE.txt
4+
* Copyright 2014 Edgar Espina
5+
*/
6+
package io.jooby.internal.pac4j;
7+
8+
import java.time.Instant;
9+
import java.util.Map;
10+
11+
import edu.umd.cs.findbugs.annotations.NonNull;
12+
import edu.umd.cs.findbugs.annotations.Nullable;
13+
import io.jooby.*;
14+
import io.jooby.pac4j.Pac4jUntrustedDataFound;
15+
16+
class Pac4jSession implements Session {
17+
public static final String PAC4J = "p4j~";
18+
19+
public static final String BIN = "b64~";
20+
21+
private final Session session;
22+
23+
public Pac4jSession(@NonNull Session session) {
24+
this.session = session;
25+
}
26+
27+
@Nullable public String getId() {
28+
return session.getId();
29+
}
30+
31+
@NonNull public Value get(@NonNull String name) {
32+
return session.get(name);
33+
}
34+
35+
@NonNull public Instant getLastAccessedTime() {
36+
return session.getLastAccessedTime();
37+
}
38+
39+
public void destroy() {
40+
session.destroy();
41+
}
42+
43+
@NonNull public Session setId(String id) {
44+
session.setId(id);
45+
return this;
46+
}
47+
48+
@NonNull public Value remove(@NonNull String name) {
49+
return session.remove(name);
50+
}
51+
52+
public boolean isNew() {
53+
return session.isNew();
54+
}
55+
56+
@NonNull public Session setNew(boolean isNew) {
57+
session.setNew(isNew);
58+
return this;
59+
}
60+
61+
@NonNull public Session setLastAccessedTime(@NonNull Instant lastAccessedTime) {
62+
session.setLastAccessedTime(lastAccessedTime);
63+
return this;
64+
}
65+
66+
public boolean isModify() {
67+
return session.isModify();
68+
}
69+
70+
@NonNull public Session setCreationTime(@NonNull Instant creationTime) {
71+
session.setCreationTime(creationTime);
72+
return this;
73+
}
74+
75+
@NonNull public Session setModify(boolean modify) {
76+
session.setModify(modify);
77+
return this;
78+
}
79+
80+
public Session renewId() {
81+
session.renewId();
82+
return this;
83+
}
84+
85+
@NonNull public Instant getCreationTime() {
86+
return session.getCreationTime();
87+
}
88+
89+
@NonNull public Map<String, String> toMap() {
90+
return session.toMap();
91+
}
92+
93+
public Session clear() {
94+
session.clear();
95+
return this;
96+
}
97+
98+
public Session getSession() {
99+
return session;
100+
}
101+
102+
public static Context create(Context ctx) {
103+
return new ForwardingContext(ctx) {
104+
@NonNull @Override
105+
public Session session() {
106+
return new Pac4jSession(super.session());
107+
}
108+
109+
@Override
110+
public Session sessionOrNull() {
111+
Session session = super.sessionOrNull();
112+
return session == null ? null : new Pac4jSession(session);
113+
}
114+
};
115+
}
116+
117+
@NonNull @Override
118+
public Session put(@NonNull String name, @NonNull String value) {
119+
if (value != null) {
120+
if (value.startsWith(PAC4J) || value.startsWith(BIN)) {
121+
throw new Pac4jUntrustedDataFound(name);
122+
}
123+
}
124+
return session.put(name, value);
125+
}
126+
}

‎modules/jooby-pac4j/src/main/java/io/jooby/internal/pac4j/SessionStoreImpl.java

+17-38
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,10 @@
1313
import static io.jooby.StatusCode.SEE_OTHER_CODE;
1414
import static io.jooby.StatusCode.TEMPORARY_REDIRECT_CODE;
1515
import static io.jooby.StatusCode.UNAUTHORIZED_CODE;
16+
import static io.jooby.internal.pac4j.Pac4jSession.BIN;
17+
import static io.jooby.internal.pac4j.Pac4jSession.PAC4J;
1618

17-
import java.io.ByteArrayInputStream;
18-
import java.io.ByteArrayOutputStream;
19-
import java.io.IOException;
20-
import java.io.ObjectInputStream;
21-
import java.io.ObjectOutputStream;
22-
import java.util.Base64;
19+
import java.io.*;
2320
import java.util.Optional;
2421

2522
import org.pac4j.core.context.WebContext;
@@ -35,19 +32,15 @@
3532
import org.pac4j.core.exception.http.UnauthorizedAction;
3633
import org.pac4j.core.exception.http.WithContentAction;
3734
import org.pac4j.core.exception.http.WithLocationAction;
35+
import org.pac4j.core.util.serializer.Serializer;
3836

3937
import io.jooby.Context;
4038
import io.jooby.Session;
41-
import io.jooby.SneakyThrows;
4239
import io.jooby.Value;
4340
import io.jooby.pac4j.Pac4jContext;
4441

4542
public class SessionStoreImpl implements org.pac4j.core.context.session.SessionStore {
4643

47-
private static final String PAC4J = "p4j~";
48-
49-
private static final String BIN = "b64~";
50-
5144
private Session getSession(WebContext context) {
5245
return context(context).session();
5346
}
@@ -75,20 +68,17 @@ public Optional<String> getSessionId(WebContext context, boolean createSession)
7568

7669
@Override
7770
public Optional<Object> get(WebContext context, String key) {
78-
Optional sessionValue =
79-
getSessionOrEmpty(context)
80-
.map(session -> session.get(key))
81-
.map(SessionStoreImpl::strToObject)
82-
.orElseGet(Optional::empty);
83-
return sessionValue;
71+
return getSessionOrEmpty(context)
72+
.map(session -> session.get(key))
73+
.flatMap(value -> strToObject(context(context).require(Serializer.class), value));
8474
}
8575

8676
@Override
8777
public void set(WebContext context, String key, Object value) {
88-
if (value == null || value.toString().length() == 0) {
78+
if (value == null || value.toString().isEmpty()) {
8979
getSessionOrEmpty(context).ifPresent(session -> session.remove(key));
9080
} else {
91-
String encoded = objToStr(value);
81+
String encoded = objToStr(context(context).require(Serializer.class), value);
9282
getSession(context).put(key, encoded);
9383
}
9484
}
@@ -116,42 +106,31 @@ public Optional<SessionStore> buildFromTrackableSession(
116106

117107
@Override
118108
public boolean renewSession(WebContext context) {
119-
getSessionOrEmpty(context).ifPresent(session -> session.renewId());
120-
return true;
109+
var session = getSessionOrEmpty(context);
110+
session.ifPresent(Session::renewId);
111+
return session.isPresent();
121112
}
122113

123-
static Optional<Object> strToObject(final Value node) {
114+
static Optional<Object> strToObject(Serializer serializer, Value node) {
124115
if (node.isMissing()) {
125116
return Optional.empty();
126117
}
127118
String value = node.value();
128119
if (value.startsWith(BIN)) {
129-
try {
130-
byte[] bytes = Base64.getDecoder().decode(value.substring(BIN.length()));
131-
return Optional.of(new ObjectInputStream(new ByteArrayInputStream(bytes)).readObject());
132-
} catch (Exception x) {
133-
throw SneakyThrows.propagate(x);
134-
}
120+
return Optional.of(serializer.deserializeFromString(value.substring(BIN.length())));
135121
} else if (value.startsWith(PAC4J)) {
136122
return Optional.of(strToAction(value.substring(PAC4J.length())));
137123
}
138124
return Optional.of(value);
139125
}
140126

141-
static String objToStr(final Object value) {
127+
static String objToStr(Serializer serializer, Object value) {
142128
if (value instanceof CharSequence || value instanceof Number || value instanceof Boolean) {
143129
return value.toString();
144130
} else if (value instanceof HttpAction) {
145131
return actionToStr((HttpAction) value);
146-
}
147-
try {
148-
ByteArrayOutputStream bytes = new ByteArrayOutputStream();
149-
ObjectOutputStream stream = new ObjectOutputStream(bytes);
150-
stream.writeObject(value);
151-
stream.flush();
152-
return BIN + Base64.getEncoder().encodeToString(bytes.toByteArray());
153-
} catch (IOException x) {
154-
throw SneakyThrows.propagate(x);
132+
} else {
133+
return BIN + serializer.serializeToString(value);
155134
}
156135
}
157136

‎modules/jooby-pac4j/src/main/java/io/jooby/internal/pac4j/UntrustedSessionDataDetector.java

+23
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
/*
2+
* Jooby https://jooby.io
3+
* Apache License Version 2.0 https://jooby.io/LICENSE.txt
4+
* Copyright 2014 Edgar Espina
5+
*/
6+
package io.jooby.internal.pac4j;
7+
8+
import edu.umd.cs.findbugs.annotations.NonNull;
9+
import io.jooby.Route;
10+
import io.jooby.Session;
11+
12+
public class UntrustedSessionDataDetector implements Route.Filter {
13+
@Override
14+
@NonNull public Route.Handler apply(@NonNull Route.Handler next) {
15+
return ctx -> {
16+
Session session = ctx.sessionOrNull();
17+
if (session instanceof Pac4jSession) {
18+
return session;
19+
}
20+
return session == null ? next.apply(ctx) : next.apply(Pac4jSession.create(ctx));
21+
};
22+
}
23+
}

‎modules/jooby-pac4j/src/main/java/io/jooby/pac4j/Pac4jModule.java

+6-1
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import org.pac4j.core.http.adapter.HttpActionAdapter;
3232
import org.pac4j.core.http.url.UrlResolver;
3333
import org.pac4j.core.profile.factory.ProfileManagerFactory;
34+
import org.pac4j.core.util.serializer.Serializer;
3435
import org.pac4j.http.client.indirect.FormClient;
3536
import org.pac4j.http.credentials.authenticator.test.SimpleTestUsernamePasswordAuthenticator;
3637

@@ -345,7 +346,10 @@ public Pac4jModule client(
345346

346347
@Override
347348
public void install(@NonNull Jooby app) throws Exception {
348-
app.getServices().putIfAbsent(Pac4jOptions.class, options);
349+
var services = app.getServices();
350+
services.putIfAbsent(Pac4jOptions.class, options);
351+
// Set defaults:
352+
services.putIfAbsent(Serializer.class, options.getSerializer());
349353

350354
var clients =
351355
ofNullable(options.getClients())
@@ -488,6 +492,7 @@ public void install(@NonNull Jooby app) throws Exception {
488492
if (securityLogic == null) {
489493
options.setSecurityLogic(newSecurityLogic(excludes));
490494
}
495+
app.use(new UntrustedSessionDataDetector());
491496

492497
/** For each client to a specific path, add a security handler. */
493498
for (var entry : allClients.entrySet()) {

‎modules/jooby-pac4j/src/main/java/io/jooby/pac4j/Pac4jOptions.java

+27-1
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
import java.util.Optional;
99

1010
import org.pac4j.core.config.Config;
11+
import org.pac4j.core.util.serializer.JavaSerializer;
12+
import org.pac4j.core.util.serializer.Serializer;
1113

1214
import edu.umd.cs.findbugs.annotations.NonNull;
1315
import edu.umd.cs.findbugs.annotations.Nullable;
@@ -52,6 +54,8 @@ public class Pac4jOptions extends Config {
5254

5355
private boolean forceLogoutRoutes = false;
5456

57+
private Serializer serializer = new JavaSerializer();
58+
5559
private Pac4jOptions(Config config) {
5660
setClients(config.getClients());
5761
Optional.ofNullable(config.getAuthorizers()).ifPresent(this::setAuthorizers);
@@ -353,8 +357,30 @@ public boolean isForceLogoutRoutes() {
353357
*
354358
* @param logoutUrlPattern It’s the logout URL pattern that the url parameter must match. It is an
355359
* optional parameter and only relative URLs are allowed by default.
360+
* @return This instance.
356361
*/
357-
public void setLogoutUrlPattern(String logoutUrlPattern) {
362+
public @NonNull Pac4jOptions setLogoutUrlPattern(String logoutUrlPattern) {
358363
this.logoutUrlPattern = logoutUrlPattern;
364+
return this;
365+
}
366+
367+
/**
368+
* Used for save complex object into session while using indirect clients.
369+
*
370+
* @return Serializer, defaults to {@link JavaSerializer}.
371+
*/
372+
public @NonNull Serializer getSerializer() {
373+
return serializer;
374+
}
375+
376+
/**
377+
* Set serializer for saving complex object into session while using indirect clients.
378+
*
379+
* @param serializer Serializer.
380+
* @return This instance.
381+
*/
382+
public @NonNull Pac4jOptions setSerializer(@NonNull Serializer serializer) {
383+
this.serializer = serializer;
384+
return this;
359385
}
360386
}

‎modules/jooby-pac4j/src/main/java/io/jooby/pac4j/Pac4jUntrustedDataFound.java

+20
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
/*
2+
* Jooby https://jooby.io
3+
* Apache License Version 2.0 https://jooby.io/LICENSE.txt
4+
* Copyright 2014 Edgar Espina
5+
*/
6+
package io.jooby.pac4j;
7+
8+
import io.jooby.StatusCode;
9+
import io.jooby.exception.StatusCodeException;
10+
11+
/**
12+
* Occurs when sensitive encoded data is set outside pac4j internals.
13+
*
14+
* @since 2.16.6
15+
*/
16+
public class Pac4jUntrustedDataFound extends StatusCodeException {
17+
public Pac4jUntrustedDataFound(String message) {
18+
super(StatusCode.FORBIDDEN, message);
19+
}
20+
}

‎modules/jooby-pac4j/src/main/java/module-info.java

+1
Original file line numberDiff line numberDiff line change
@@ -13,4 +13,5 @@
1313
requires org.slf4j;
1414
requires pac4j.core;
1515
requires pac4j.http;
16+
requires jsr305;
1617
}

0 commit comments

Comments
 (0)