Security: fromJson() performs unrestricted reflective field binding (mass assignment); Javadoc claims a non-existent allowlist
Nessuno ha ancora preso questa issue.
Valutazione
- Difficoltà
- 5/5
- Tempo stimato
- Più di una settimana
- Idoneità per principianti
- 35/100
Direzione di ricerca
Start in src/main/java/org/json/JSONObject.java at fromJson(Class) and the static fromJson(String, Class), then compare deserialization with the serialize-side handling of transient fields and @JSONPropertyIgnore. Confirm the preferred binding policy with maintainers; done means the agreed security behavior is implemented, the Javadoc matches it, and the supplied reproduction no longer overwrites excluded fields.
Scritto dal modello di indicizzazione a partire dal testo della issue.
Descrizione
Summary
JSONObject.fromJson(Class) (and the static fromJson(String, Class)) in current master performs unrestricted reflective field binding from untrusted JSON — a classic mass-assignment primitive. It iterates every non-static declared field, calls field.setAccessible(true) unconditionally, and writes any field whose name matches a JSON key. There is:
- no allowlist / registry (despite the Javadoc claiming one — see below),
- no
transientexclusion, - no honoring of the library's own
@JSONPropertyIgnoreopt-out (which is honored on the serialize side).
Filing as a normal issue because this API is not in any release yet (not in 20250517); ideally it's hardened or gated before it ships.
The Javadoc claims a protection that does not exist
The fromJson(Class) Javadoc states:
"Only classes that are explicitly supported and registered within the JSONObject context can be deserialized... This ensures that only a limited and controlled set of types can be instantiated from JSON for safety and predictability."
There is no such registry anywhere in the source. Every class with a no-arg constructor is eligible and every non-static field is writable.
Code (src/main/java/org/json/JSONObject.java, fromJson(Class))
T obj = clazz.getDeclaredConstructor().newInstance();
for (Field field : clazz.getDeclaredFields()) {
if (Modifier.isStatic(field.getModifiers())) {
continue; // ONLY static is skipped
}
field.setAccessible(true); // unconditional
String fieldName = field.getName();
if (has(fieldName)) {
Object value = get(fieldName);
Object convertedValue = convertValue(value, field.getGenericType());
field.set(obj, convertedValue); // writes transient / private / @JSONPropertyIgnore fields
}
}
Reproduction
import org.json.JSONObject;
import org.json.JSONPropertyIgnore;
public class Repro {
public static class UserUpdate {
public String displayName;
private boolean admin = false;
private String tenantId = "own-tenant";
private transient String sessionKey = "server-issued";
private boolean superAdmin = false;
public boolean isAdmin() { return admin; }
public String getTenantId() { return tenantId; }
public String getSessionKey() { return sessionKey; }
@JSONPropertyIgnore
public boolean isSuperAdmin() { return superAdmin; }
}
public static void main(String[] args) {
String json = "{\"displayName\":\"bob\",\"admin\":true,\"tenantId\":\"victim-tenant\","
+ "\"sessionKey\":\"attacker-supplied\",\"superAdmin\":true}";
UserUpdate u = JSONObject.fromJson(json, UserUpdate.class);
System.out.println("admin=" + u.isAdmin() + " tenantId=" + u.getTenantId()
+ " sessionKey=" + u.getSessionKey() + " superAdmin=" + u.isSuperAdmin());
}
}
Output:
admin=true tenantId=victim-tenant sessionKey=attacker-supplied superAdmin=true
A private boolean admin, a private String tenantId, a transient sessionKey, and a @JSONPropertyIgnore-annotated superAdmin are all overwritten from untrusted JSON and observable through the object's own getters.
Why this matters
If fromJson(requestBody, SomePojo.class) is used the way the API documents, an anonymous caller can set fields the developer never meant to expose — privilege flags, tenant-isolation fields, or values explicitly marked transient/@JSONPropertyIgnore — the same class of bug as the Rails attr_accessible and Spring DataBinder mass-assignment issues.
Suggested fix
- Skip
transientfields (Modifier.isTransient), matching the serialize side. - Honor
@JSONPropertyIgnore(or a field-level ignore) on deserialize. - Don't unconditionally
setAccessible(true); default to public fields or require an explicit opt-in for private-field binding. - Fix the Javadoc so it no longer claims a registry/allowlist that isn't implemented.
Happy to open a PR if the maintainers agree on the preferred approach (skip transient + honor @JSONPropertyIgnore, vs. an opt-in binding model).
- Lingua principale
- Java
- Stelle
- 4.7k
- Fork
- 2.6k
- Merge medio
- 11g 18m
- PR unite (30g)
- 1
Guida per i contributori
Apri la guida per i contributori
Come iniziare
- Leggi tutta la issue e poi la guida ai contributi del progetto.
- Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
- Fai un fork del repository e lavora su un branch.
- Apri una pull request che faccia riferimento al numero della issue.
Altre issue di stleary/JSON-java
-
Fix before the next release
Difficoltà 2/5 1-3 ore Idoneità per principianti 78/100
-
New JSONPointer tests needed Aperta
Difficoltà 2/5 1-3 ore Idoneità per principianti 76/100
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 88/100
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 82/100
-
Fix before the next release
Difficoltà 2/5 1-3 ore Idoneità per principianti 78/100
Tutte le issue di stleary/JSON-java
Issue simili
-
Bug Java Platform: Java
Difficoltà 2/5 1-3 ore Idoneità per principianti 78/100
getsentry/sentry-java#6138 · 1 commento ·
-
bug needs triage p2
Difficoltà 2/5 1-3 ore Idoneità per principianti 78/100
GoogleCloudPlatform/DataflowTemplates#4273 · 1 commento ·
-
[Studio][Bug] Bulk-deleting a full page of alert rules steps the page back while more rules remain Aperta
Difficoltà 2/5 1-3 ore Idoneità per principianti 78/100
apache/rocketmq-dashboard#4654 · 1 commento ·
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 78/100
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 76/100