World

TX Proxy Relayer

6.1 Unbounded request size allows resource exhaustion and denial of service

Upon receiving an HTTP request, the from_request is invoked to parse and read the body:

pub async fn from_request(request: http::Request<HttpBody>) -> Result<Self> {
    let (parts, body) = request.into_parts();
    let (body_bytes, _) = http_helpers::read_body(&parts.headers, body, u32::MAX).await?;
    let method = serde_json::from_slice::<Request>(&body_bytes)?
        .method
        .to_string();

    Ok(Self {
        parts,
        body: body_bytes,
        method,
    })
}

The current implementation accepts payloads up to u32::MAX (approximately 4 GB), which is unnecessarily large for typical RPC requests. This exposes the server to a denial-of-service (DoS) attack, where a malicious user can send repeated large requests to consume memory and processing resources, potentially degrading service availability.

Recommendation

RPC requests are typically lightweight and rarely exceed a few megabytes. It is recommended to set a much lower and reasonable upper limit on the request body size

6.2 Absence of rate limiting allows malicious users to monopolize the maximum concurrent connections

While the server is configured to enforce a maximum number of concurrent connections, no rate limiting mechanism is implemented to regulate incoming traffic:

pub async fn run(self) -> Result<()> {
    // --SNIP
    let server = Server::builder()
        .set_http_middleware(middleware)
        .max_connections(self.max_concurrent_connections)
        .build(format!("{}:{}", self.http_addr, self.http_port))
        .await?;
}

The rate limit is a strategy for limiting network traffic. It puts a cap on how often an IP address can repeat an action within a certain timeframe. The lack of rate limiting allows a single IP address to continuously send requests without restriction, exhausting all available concurrent connections. This can lead to a denial of service (DoS) for legitimate users.

Resolution

The service is intended to run behind IP whitelisting and is only accessible within permissioned infrastructure. Since it is expected to receive high request volumes from a small set of IP addresses, rate limiting should be handled elsewhere in the stack.

6.3 Clients are allowed to use HTTP, exposing JWT tokens

When initializing HTTP clients for the builder and L2 backends, the configuration permits both HTTPS and HTTP traffic:

impl HttpClient {
    pub fn new(url: Uri, secret: JwtSecret) -> Self {
        let connector = hyper_rustls::HttpsConnectorBuilder::new()
            .with_native_roots()
            .expect("no native root CA certificates found")
==>            .https_or_http()
            .enable_http1()
            .enable_http2()
            .build();
    }
}

On each request, a JWT token is attached for authentication:

let client = Client::builder(TokioExecutor::new()).build(connector);

let client = ServiceBuilder::new()
    .layer(DecompressionLayer::new())
    .layer(AuthClientLayer::new(secret))
    .service(client);

Self { client, url }

Allowing HTTP traffic poses a significant security risk, as HTTP does not encrypt requests. In non-local deployments—where the builder and L2 backends are hosted on separate servers—an attacker could intercept HTTP traffic and extract the JWT token, compromising the authentication mechanism.

Recommendation

The recommended approach is to explicitly disable HTTP support by requiring HTTPS connections only. If HTTP support is necessary for specific environments, consider encrypting the JWT token on the request.

Resolution

The service is intended to run within a permissioned infrastructure where all network traffic is secured via TLS. While HTTP is supported to enable local testing, production deployments are expected to use HTTPS exclusively. JWTs are only transmitted over these secure connections.

6.4 HTTP clients are instantiated without applying timeout

For each backend, three instances of an HTTP client are created. Each instance is expected to enforce a timeout value, which is configurable via a command-line flag during backend initialization to prevent potential resource exhaustion:

macro_rules! define_rpc_args {
    ($(($name:ident, $prefix:ident)),*) => {
        // --SNIP
        #[arg(long, env, default_value_t = 1000)]
        pub [<$prefix _timeout>]: u64,
    }
}

However, the configured timeout is not applied when initializing the HTTP clients:

pub fn build(&self) -> Result<FanoutWrite> {
    let jwt = self.get_jwt()?;
    let client_0 = HttpClient::new(self.[<$prefix _url_0>].clone(), jwt.clone());
    let client_1 = HttpClient::new(self.[<$prefix _url_1>].clone(), jwt.clone());
    let client_2 = HttpClient::new(self.[<$prefix _url_2>].clone(), jwt);
    Ok(FanoutWrite::new(vec![client_0, client_1, client_2]))
}

This omission could lead to unbounded request times, risking performance degradation or backend overload.

Recommendation

Consider applying the timeout when initializing the HTTP clients.

6.5 The graceful shutdown is not handled completely.

After the relayer server is started, a Tokio select! block is used to listen for scenarios where the server should shut down gracefully by invoking the handle's stop() method:

pub async fn run(self) -> Result<()> {
    // --SNIP
    let handle = server.start(RpcModule::new(()));

    let stopped_handle = handle.clone();
    let shutdown_handle = handle.clone();

    tokio::select! {
        _ = stopped_handle.stopped() => {
            error!("Server stopped unexpectedly or crashed");
            Err(eyre::eyre!("Server stopped unexpectedly or crashed"))
    }
        _ = tokio::signal::ctrl_c() => {
            error!("Received Ctrl-C, shutting down...");
            shutdown_handle.stop()?;
            Ok(())
        }
    }
}

The current implementation does not handle the SIGTERM signal, which is commonly used by process managers and container orchestrators in production environments to trigger a graceful shutdown. Since SIGTERM is not captured, the server may be terminated abruptly, skipping the invocation of handle.stop() and potentially resulting in unclean resource cleanup or data corruption.

Recommendation

Consider adding the SIGTERM arm in the select! block:

pub async fn run(self) -> Result<()> {
    // --SNIP
    let handle = server.start(RpcModule::new(()));
    
+   let mut sigterm = signal(SignalKind::terminate()).unwrap();
    
    let stopped_handle = handle.clone();
    let shutdown_handle = handle.clone();
+   let force_handle = handle.clone();

    tokio::select! {
        _ = stopped_handle.stopped() => {
            error!("Server stopped unexpectedly or crashed");
            Err(eyre::eyre!("Server stopped unexpectedly or crashed"))
    }
        _ = tokio::signal::ctrl_c() => {
            error!("Received Ctrl-C, shutting down...");
            shutdown_handle.stop()?;
            Ok(())
        }
+       _ = sigterm.recv() => {
+           error!("Forcefully shutdown by process manager");
+           force_handle.stop()?;
+           Ok(())
+       }
    }
}

6.6 Metrics server could fail to start, but won't stop the server

During server initialization, the Prometheus metrics server is launched asynchronously in a separate tokio task. However, this task’s execution status is not monitored or validated. Consequently, any failures occurring during initialization of the metrics server would remain undetected, more specifically if the address passed is already bound to a TCP listener, potentially causing degraded observability or unexpected behavior. Furthermore, such initialization failures do not prevent the main server from continuing execution, potentially masking critical operational errors.

1) We initialize our metrics in a separate tokio task in cli::Cli::init_metrics

fn init_metrics(&self) -> Result<()> {
    if self.metrics {
        ...
        tokio::spawn(init_metrics_server(addr, handle)); // Run the metrics server in a separate task
    }

    Ok(())
}

2) init_metrics_server function starts the server, and in the case of errors, returns it as a eyre::Result<()>, which is not handled properly after

pub(crate) async fn init_metrics_server(
    addr: SocketAddr,
    handle: PrometheusHandle,
) -> eyre::Result<()> {
    let listener = TcpListener::bind(addr).await?;
    info!("Metrics server running on {}", addr);

    loop {
        match listener.accept().await {
            Ok((stream, _)) => {
                let handle = handle.clone(); // Clone the handle for each connection
                tokio::task::spawn(async move {
                    let service = service_fn(move |_req: Request<hyper::body::Incoming>| {
                        let response = match _req.uri().path() {
                            "/metrics" => Response::builder()
                                .header("content-type", "text/plain")
                                .body(HttpBody::from(handle.render()))
                                .unwrap(),
                            _ => Response::builder()
                                .status(StatusCode::NOT_FOUND)
                                .body(HttpBody::empty())
                                .unwrap(),
                        };
                        async { Ok::<_, hyper::Error>(response) }
                    });

                    let io = TokioIo::new(stream);

                    if let Err(err) = http1::Builder::new().serve_connection(io, service).await {
                        error!(message = "Error serving metrics connection", error = %err);
                    }
                });
            }
            Err(e) => {
                error!(message = "Error accepting connection", error = %e);
            }
        }
    }
}

Recommendation

Consider reworking the metrics server initialization such that its failure is observed and handled accordingly. Instead of spawning init_metrics_server as an unbound task, you could wait on its return in a supervised task or use a special error channel (e.g., a oneshot::Sender) to return initialization failures to the main thread. That would allow the application to fail fast or log critical startup failures before it carries on. Alternatively, use a task supervisor pattern (e.g., tokio::task::JoinHandle with explicit .await and error propagation) or structured concurrency utilities like tokio::select! with cancellation guards to monitor and manage task-level failures such that observability is not silently lost.

6.7 Address parsing in metrics server fails for IPv6 addresses

During metrics server initialization, the address is constructed using string formatting with ::

let server = Server::builder()
    .set_http_middleware(middleware)
    .max_connections(self.max_concurrent_connections)
    .build(format!("{}:{}", self.http_addr, self.http_port))
    .await?;

fn init_metrics(&self) -> Result<()> {
    if self.metrics {
        let recorder = PrometheusBuilder::new().build_recorder();
        let handle = recorder.handle();

        Stack::new(recorder)
            .push(PrefixLayer::new("tx-proxy"))
            .install()?;

        // Start the metrics server
==>        let metrics_addr = format!("{}:{}", self.metrics_host, self.metrics_port);
        let addr: SocketAddr = metrics_addr.parse()?;
        tokio::spawn(init_metrics_server(addr, handle)); // Run the metrics server in a separate task
    }

    Ok(())
}

This approach fails for IPv6 addresses, as the resulting string (e.g., ::1:9090) is not a valid representation and will cause parsing errors.

Recommendation

SocketAddr already exposes a new function to build an address from the ip and the port directly, consider using it.

6.8 Flags cannot be parsed from .env file

The server's configuration flags are expected to be loaded from environment variables. Based on the project structure and the presence of a .env file, it is also expected that these flags can be loaded automatically from the file. However, the .env file is not explicitly loaded in the main function.

While clap's #[arg(env)] attribute can pick up environment variables already set in the shell session, it does not load a .env file by itself. As a result, variables defined only in .env will not be available when parsing the CLI arguments.

Recommendation

Consider using the dotenv crate and calling it at the very beginning of the main function

6.9 Missing instrument in ValidationService

Unlike forward function in HttpClient, the call function in ValidationService manually injects target = tx-proxy::validation into each debug! statement. This approach is more verbose and prone to inconsistencies. Applying #[instrument] would automatically apply span for all following traces.

Recommendation

Consider adding the #[instrument] macro to the call function, similarly to how it is used in forward.

Example of a possible solution:

#[instrument(
    skip(self, request),
    target = "tx-proxy::validation",
)]
fn call(&mut self, request: HttpRequest<HttpBody>) -> Self::Future {

6.10 Mixing HttpBody with Hyper may cause future compatibility issues

The type jsonrpsee::http_client::HttpBody is used as the body type in the metrics HTTP handler, even though the handler itself is built using the Hyper crate. While this currently works, it creates a fragile dependency: if the internal implementation of HttpBody changes in future versions of jsonrpsee, the metrics server could break unexpectedly.

Recommendation

Consider using hyper::Body directly for HTTP handlers built with Hyper.

6.11 max_concurrent_connections cannot be configured via environment variables

The project uses the clap crate to fetch configuration values from either CLI arguments or environment variables. Although MAX_CONCURRENT_CONNECTIONS is defined in the .env file, the server currently does not accept its value from the environment due to the missing env attribute in the clap macro.

Recommendation

Consider adding env attribute to clap macro:

    #[clap(long = "http.max-concurrent-connections", default_value_t = 500)
+   ,env 
    ]
    pub max_concurrent_connections: u32,

6.12 cargo-audit: crossbeam-channel: double free on Drop

The project indirectly depends on crossbeam-channel version 0.5.14, which is affected by a __double free on Drop__ vulnerability. This vulnerability can cause memory corruption and undefined behaviour. It was disclosed on 2025-04-08 and tracked under RUSTSEC-2025-0024.

To reproduce this issue, install cargo-audit locally and run cargo audit in the project's directory.

The dependency path is: crossbeam-channel 0.5.14 -> moka 0.12.10 -> rollup-boost 0.1.0 -> tx-proxy 0.1.0

Recommendation

Consider updating dependencies to the versions that mitigate this issue. Also, as a possible solution, consider updating Cargo.lock by removing it and running cargo build once again.

6.13 Tracing should be configured to write logs to a file

The proxy relayer server is configured to trace incoming requests upon startup. However, the tracer is not configured with a custom writer, and thus defaults to writing logs to stdout:

impl<S> Default for Layer<S> {
    fn default() -> Self {
        // only enable ANSI when the feature is enabled, and the NO_COLOR
        // environment variable is unset or empty.
        let ansi = cfg!(feature = "ansi") && env::var("NO_COLOR").map_or(true, |v| v.is_empty());

        Layer {
            fmt_fields: format::DefaultFields::default(),
            fmt_event: format::Format::default(),
            fmt_span: format::FmtSpanConfig::default(),
            make_writer: io::stdout,
            is_ansi: ansi,
            log_internal_errors: false,
            _inner: PhantomData,
        }
    }
}

Writing logs solely to the terminal ( stdout) means that all tracing information is lost if the server crashes or is restarted. Persisting logs to a file ensures that important operational and diagnostic information is retained across restarts.

Recommendation

Consider configuring a writer (File interface) to the tracing subscriber

6.14 Unnecessary cloning

In the build function of define_rpc_args macro implementation, the jwt token is cloned multiple times when constructing HttpClient instances:

pub fn build(&self) -> Result<FanoutWrite> {
    let jwt = self.get_jwt()?;
    let client_0 = HttpClient::new(self.[<$prefix _url_0>].clone(), jwt.clone());
    let client_1 = HttpClient::new(self.[<$prefix _url_1>].clone(), jwt.clone());
    let client_2 = HttpClient::new(self.[<$prefix _url_2>].clone(), jwt);
    Ok(FanoutWrite::new(vec![client_0, client_1, client_2]))
}

However, cloning jwt is unnecessary, since JwtToken implements the Copy trait, so we can simply pass it as a value.

Recommendation

Consider removing .clone():

pub fn build(&self) -> Result<FanoutWrite> {
    let jwt = self.get_jwt()?;
    let client_0 = HttpClient::new(self.[<$prefix _url_0>].clone(), jwt);
    let client_1 = HttpClient::new(self.[<$prefix _url_1>].clone(), jwt);
    let client_2 = HttpClient::new(self.[<$prefix _url_2>].clone(), jwt);
    Ok(FanoutWrite::new(vec![client_0, client_1, client_2]))
}