Skip to content

Commit 7047746

Browse files
Dyslex7chanabi1224sudo-shashank
authored
feat(rpc): implement duplicate key detection for json-rpc requests (#6455)
Co-authored-by: hanabi1224 <[email protected]> Co-authored-by: Shashank <[email protected]>
1 parent 7bffdd6 commit 7047746

File tree

7 files changed

+310
-0
lines changed

7 files changed

+310
-0
lines changed

.config/forest.dic

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ daemonize
3939
Datacap
4040
devnet
4141
DB/S
42+
deduplicates
4243
deserialize/D
4344
destructuring
4445
DNS
@@ -70,6 +71,8 @@ IPFS
7071
ip
7172
IPLD
7273
JSON
74+
jsonrpc
75+
jsonrpsee
7376
JWT
7477
Kademlia
7578
Kubernetes
@@ -110,6 +113,7 @@ SECP256k1
110113
seekable
111114
serializable
112115
serializer/SM
116+
serde_json
113117
skippable
114118
statediff
115119
stateful

Cargo.lock

Lines changed: 7 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,7 @@ is-terminal = "0.4"
115115
itertools = "0.14"
116116
jsonrpsee = { version = "0.26", features = ["server", "ws-client", "http-client", "macros"] }
117117
jsonwebtoken = { version = "10", features = ["aws_lc_rs"] }
118+
justjson = "0.3"
118119
keccak-hash = "0.12"
119120
kubert-prometheus-process = "0.2"
120121
lazy-regex = "3"

docs/docs/users/reference/env_variables.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ process.
5454
| `FOREST_JWT_DISABLE_EXP_VALIDATION` | 1 or true | empty | 1 | Whether or not to disable JWT expiration validation |
5555
| `FOREST_ETH_BLOCK_CACHE_SIZE` | positive integer | 500 | 1 | The size of Eth block cache |
5656
| `FOREST_RPC_BACKFILL_FULL_TIPSET_FROM_NETWORK` | 1 or true | false | 1 | Whether or not to backfill full tipsets from the p2p network |
57+
| `FOREST_STRICT_JSON` | 1 or true | false | 1 | Enable strict JSON validation to detect duplicate keys in RPC requests |
5758

5859
### `FOREST_F3_SIDECAR_FFI_BUILD_OPT_OUT`
5960

src/rpc/json_validator.rs

Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,158 @@
1+
// Copyright 2019-2026 ChainSafe Systems
2+
// SPDX-License-Identifier: Apache-2.0, MIT
3+
4+
//! JSON validation utilities for detecting duplicate keys before serde_json processing.
5+
//!
6+
//! serde_json automatically deduplicates keys at parse time using a "last-write-wins" strategy
7+
//! This means JSON like `{"/":"cid1", "/":"cid2"}` will keep only the last value, which can lead to unexpected behavior in RPC calls.
8+
9+
use ahash::HashSet;
10+
use justjson::Value;
11+
12+
pub const STRICT_JSON_ENV: &str = "FOREST_STRICT_JSON";
13+
14+
#[cfg(not(test))]
15+
use std::sync::LazyLock;
16+
17+
#[cfg(not(test))]
18+
static STRICT_MODE: LazyLock<bool> =
19+
LazyLock::new(|| crate::utils::misc::env::is_env_truthy(STRICT_JSON_ENV));
20+
21+
#[inline]
22+
pub fn is_strict_mode() -> bool {
23+
#[cfg(test)]
24+
{
25+
crate::utils::misc::env::is_env_truthy(STRICT_JSON_ENV)
26+
}
27+
#[cfg(not(test))]
28+
{
29+
*STRICT_MODE
30+
}
31+
}
32+
33+
/// validates JSON for duplicate keys by parsing at the token level.
34+
pub fn validate_json_for_duplicates(json_str: &str) -> Result<(), String> {
35+
if !is_strict_mode() {
36+
return Ok(());
37+
}
38+
39+
fn check_value(value: &Value) -> Result<(), String> {
40+
match value {
41+
Value::Object(obj) => {
42+
let mut seen = HashSet::default();
43+
for entry in obj.iter() {
44+
let key = entry
45+
.key
46+
.as_str()
47+
.ok_or_else(|| "Invalid JSON key".to_string())?;
48+
49+
if !seen.insert(key) {
50+
return Err(format!(
51+
"duplicate key '{}' in JSON object - this likely indicates malformed input. \
52+
Set {}=0 to disable this check",
53+
key, STRICT_JSON_ENV
54+
));
55+
}
56+
check_value(&entry.value)?;
57+
}
58+
Ok(())
59+
}
60+
Value::Array(arr) => {
61+
for item in arr.iter() {
62+
check_value(item)?;
63+
}
64+
Ok(())
65+
}
66+
_ => Ok(()),
67+
}
68+
}
69+
// defer to serde_json for invalid JSON
70+
let value = match Value::from_json(json_str) {
71+
Ok(v) => v,
72+
Err(_) => return Ok(()),
73+
};
74+
check_value(&value)
75+
}
76+
77+
#[cfg(test)]
78+
mod tests {
79+
use super::*;
80+
use serial_test::serial;
81+
82+
fn with_strict_mode<F>(enabled: bool, f: F)
83+
where
84+
F: FnOnce(),
85+
{
86+
let original = std::env::var(STRICT_JSON_ENV).ok();
87+
88+
if enabled {
89+
unsafe {
90+
std::env::set_var(STRICT_JSON_ENV, "1");
91+
}
92+
} else {
93+
unsafe {
94+
std::env::remove_var(STRICT_JSON_ENV);
95+
}
96+
}
97+
98+
f();
99+
100+
unsafe {
101+
match original {
102+
Some(val) => std::env::set_var(STRICT_JSON_ENV, val),
103+
None => std::env::remove_var(STRICT_JSON_ENV),
104+
}
105+
}
106+
}
107+
108+
#[test]
109+
#[serial]
110+
fn test_no_duplicates() {
111+
with_strict_mode(true, || {
112+
let json = r#"{"a": 1, "b": 2, "c": 3}"#;
113+
assert!(validate_json_for_duplicates(json).is_ok());
114+
});
115+
}
116+
117+
#[test]
118+
#[serial]
119+
fn test_duplicate_keys_detected() {
120+
with_strict_mode(true, || {
121+
let json = r#"{"/":"cid1", "/":"cid2"}"#;
122+
let result = validate_json_for_duplicates(json);
123+
assert!(result.is_err(), "Should have detected duplicate key");
124+
assert!(result.unwrap_err().contains("duplicate key"));
125+
});
126+
}
127+
128+
#[test]
129+
#[serial]
130+
fn test_strict_mode_disabled() {
131+
with_strict_mode(false, || {
132+
// should pass with strict mode disabled
133+
let json = r#"{"/":"cid1", "/":"cid2"}"#;
134+
assert!(validate_json_for_duplicates(json).is_ok());
135+
});
136+
}
137+
138+
#[test]
139+
#[serial]
140+
fn test_duplicate_cid_keys() {
141+
with_strict_mode(true, || {
142+
let json = r#"{
143+
"jsonrpc": "2.0",
144+
"id": 1,
145+
"method": "Filecoin.ChainGetMessagesInTipset",
146+
"params": [[{
147+
"/":"bafy2bzacea43254b5x6c4l22ynpjfoct5qvabbbk2abcfspfcjkiltivrlyqi",
148+
"/":"bafy2bzacea4viqyaozpfk57lnemwufryb76llxzmebxc7it2rnssqz2ljdl6a",
149+
"/":"bafy2bzaceav6j67epppz5ib55v5ty26dhkq4jinbsizq2olb3azbzxvfmc73o"
150+
}]]
151+
}"#;
152+
153+
let result = validate_json_for_duplicates(json);
154+
assert!(result.is_err());
155+
assert!(result.unwrap_err().contains("duplicate key '/'"));
156+
});
157+
}
158+
}

src/rpc/mod.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,13 @@ mod channel;
77
mod client;
88
mod filter_layer;
99
mod filter_list;
10+
pub mod json_validator;
1011
mod log_layer;
1112
mod metrics_layer;
1213
mod request;
1314
mod segregation_layer;
1415
mod set_extension_layer;
16+
mod validation_layer;
1517

1618
use crate::rpc::eth::types::RandomHexStringIdProvider;
1719
use crate::shim::clock::ChainEpoch;
@@ -623,6 +625,7 @@ where
623625
.layer(SetExtensionLayer { path })
624626
.layer(SegregationLayer)
625627
.layer(FilterLayer::new(filter_list.clone()))
628+
.layer(validation_layer::JsonValidationLayer)
626629
.layer(AuthLayer {
627630
headers,
628631
keystore: keystore.clone(),

src/rpc/validation_layer.rs

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
// Copyright 2019-2026 ChainSafe Systems
2+
// SPDX-License-Identifier: Apache-2.0, MIT
3+
4+
use futures::future::Either;
5+
use jsonrpsee::MethodResponse;
6+
use jsonrpsee::core::middleware::{Batch, BatchEntry, BatchEntryErr, Notification};
7+
use jsonrpsee::server::middleware::rpc::RpcServiceT;
8+
use jsonrpsee::types::ErrorObject;
9+
use tower::Layer;
10+
11+
use super::json_validator;
12+
13+
/// JSON-RPC error code for invalid request
14+
const INVALID_REQUEST: i32 = -32600;
15+
16+
/// stateless jsonrpsee layer for validating JSON-RPC requests
17+
#[derive(Clone, Default)]
18+
pub(super) struct JsonValidationLayer;
19+
20+
impl<S> Layer<S> for JsonValidationLayer {
21+
type Service = Validation<S>;
22+
23+
fn layer(&self, service: S) -> Self::Service {
24+
Validation { service }
25+
}
26+
}
27+
28+
#[derive(Clone)]
29+
pub(super) struct Validation<S> {
30+
service: S,
31+
}
32+
33+
impl<S> Validation<S> {
34+
fn validation_enabled() -> bool {
35+
json_validator::is_strict_mode()
36+
}
37+
38+
fn validate_params(params_str: &str) -> Result<(), String> {
39+
json_validator::validate_json_for_duplicates(params_str)
40+
}
41+
}
42+
43+
impl<S> RpcServiceT for Validation<S>
44+
where
45+
S: RpcServiceT<MethodResponse = MethodResponse, NotificationResponse = MethodResponse>
46+
+ Send
47+
+ Sync
48+
+ Clone
49+
+ 'static,
50+
{
51+
type MethodResponse = S::MethodResponse;
52+
type NotificationResponse = S::NotificationResponse;
53+
type BatchResponse = S::BatchResponse;
54+
55+
fn call<'a>(
56+
&self,
57+
req: jsonrpsee::types::Request<'a>,
58+
) -> impl Future<Output = Self::MethodResponse> + Send + 'a {
59+
if !Self::validation_enabled() {
60+
return Either::Left(self.service.call(req));
61+
}
62+
63+
if let Err(e) =
64+
json_validator::validate_json_for_duplicates(req.params().as_str().unwrap_or("[]"))
65+
{
66+
let err = ErrorObject::owned(INVALID_REQUEST, e, None::<()>);
67+
return Either::Right(async move { MethodResponse::error(req.id(), err) });
68+
}
69+
70+
Either::Left(self.service.call(req))
71+
}
72+
73+
fn batch<'a>(
74+
&self,
75+
mut batch: Batch<'a>,
76+
) -> impl Future<Output = Self::BatchResponse> + Send + 'a {
77+
let service = self.service.clone();
78+
79+
async move {
80+
if !Self::validation_enabled() {
81+
return service.batch(batch).await;
82+
}
83+
84+
for entry in batch.iter_mut() {
85+
if let Ok(batch_entry) = entry {
86+
let params_str = match batch_entry.params() {
87+
Some(p) => p.as_ref().get(),
88+
None => continue,
89+
};
90+
91+
if let Err(e) = Self::validate_params(params_str) {
92+
match batch_entry {
93+
BatchEntry::Call(req) => {
94+
let err = ErrorObject::owned(INVALID_REQUEST, e, None::<()>);
95+
let err_entry = BatchEntryErr::new(req.id().clone(), err);
96+
*entry = Err(err_entry);
97+
}
98+
BatchEntry::Notification(_notif) => {
99+
let err = ErrorObject::owned(INVALID_REQUEST, e, None::<()>);
100+
let err_entry = BatchEntryErr::new(jsonrpsee::types::Id::Null, err);
101+
*entry = Err(err_entry);
102+
}
103+
}
104+
}
105+
}
106+
}
107+
108+
service.batch(batch).await
109+
}
110+
}
111+
112+
fn notification<'a>(
113+
&self,
114+
n: Notification<'a>,
115+
) -> impl Future<Output = Self::NotificationResponse> + Send + 'a {
116+
let service = self.service.clone();
117+
118+
async move {
119+
if !Self::validation_enabled() {
120+
return service.notification(n).await;
121+
}
122+
123+
let params_str = match n.params() {
124+
Some(p) => p.as_ref().get(),
125+
None => return service.notification(n).await,
126+
};
127+
128+
if let Err(e) = Self::validate_params(params_str) {
129+
let err = ErrorObject::owned(INVALID_REQUEST, e, None::<()>);
130+
return MethodResponse::error(jsonrpsee::types::Id::Null, err);
131+
}
132+
133+
service.notification(n).await
134+
}
135+
}
136+
}

0 commit comments

Comments
 (0)