apache / apache/dubbo

Classes with writeReplace() bypass Hessian2 Serializable security check

Open
#16,287 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
Java
Stars
41.6k
Forks
26.4k
Avg merge
15h 13m
Merged PRs (30d)
4

Description

### Environment

- Dubbo version: 3.2.15 (also affects 3.3.x — the relevant code path is in hessian-lite)
- hessian-lite version: 3.2.13
- JDK: 17

### Description

`Hessian2SerializerFactory` hooks into `getDefaultSerializer()` to enforce the `checkSerializable` security check. However, in hessian-lite's `SerializerFactory.getSerializer(Class)`, classes that define a `writeReplace()` method are handled **before** `getDefaultSerializer()` is reached — they get a `JavaSerializer` directly and skip the `getDefaultSerializer` override entirely.

This means:
1. A class **without** `Serializable` and **without** `writeReplace()` → correctly rejected by `checkSerializable`
2. A class **without** `Serializable` but **with** `writeReplace()` → **bypasses the check**, serialization succeeds

The relevant priority chain in `SerializerFactory.getSerializer(Class)`:

```
1. _staticSerializerMap (built-in types)
2. Custom serializer factories
3. EnumSet check
4. writeReplace check → new JavaSerializer(cl, loader); goto cache ← short-circuits here
5. Map / Collection / Iterator / Enum / ...
6. getDefaultSerializer(cl) ← checkSerializable is here, never reached for step 4
```

### How to reproduce

Define two DTOs:

```java
// No Serializable, no writeReplace
public class NormalDto {
private String name;
// getter/setter
}

// No Serializable, HAS writeReplace
public class WriteReplaceDto {
private String name;
// getter/setter

private Object writeReplace() {
return this;
}
}
```

Define a Dubbo service that accepts both as parameters:

```java
public interface TestService {
String sendNormal(NormalDto dto);
String sendWriteReplace(WriteReplaceDto dto);
}
```

Call both methods from a consumer:

- `sendNormal(new NormalDto("test"))` → **Fails on consumer side** with `"Serialized class ... has not implement Serializable interface"`
- `sendWriteReplace(new WriteReplaceDto("test"))` → **Consumer serializes successfully** and sends the message (provider-side deserialization catches it via a different path — `loadSerializedClass`)

### Observed behavior

The security check is **inconsistent** between serialization and deserialization:

| Side | Path | writeReplace class caught? |
|------|------|---------------------------|
| Serialization (sender) | `getSerializer()` → writeReplace detected → `JavaSerializer` directly → **bypasses** `getDefaultSerializer` / `checkSerializable` | **No** |
| Deserialization (receiver) | `loadSerializedClass()` → `DefaultSerializeClassChecker.loadClass()` | **Yes** |

### Expected behavior

The `checkSerializable` enforcement should be consistent regardless of whether a class has `writeReplace()`. A class that doesn't implement `Serializable` should be rejected on both the serialization and deserialization sides.

### Suggested fix

Override `getSerializer(Class)` in `Hessian2SerializerFactory` to ensure the security check runs before delegating to `super.getSerializer()`, so that writeReplace classes are also covered. Alternatively, the check could be moved into `SerializerFactory.getSerializer(Class)` in hessian-lite itself, before the writeReplace branch.

Contributor guide

Open the contributing guide

Research direction

Start with Hessian2SerializerFactory.getDefaultSerializer(Class) and hessian-lite's SerializerFactory.getSerializer(Class), tracing the writeReplace branch before the default serializer. Reproduce the behavior with the NormalDto and WriteReplaceDto examples, then verify that a non-Serializable writeReplace class is rejected during serialization while the existing Serializable check remains consistent on deserialization.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
distributed-systems, security
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
72/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.