Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions ev-stacks/stacks/da-celestia/entrypoint.da.sh
Original file line number Diff line number Diff line change
Expand Up @@ -76,14 +76,14 @@ fi
# Ensure TxWorkerAccounts is set to 8 under [State] section
log "CONFIG" "Ensuring TxWorkerAccounts is set to 8 in [State] section"
if grep -q "^\[State\]" "$LIGHT_NODE_CONFIG_PATH"; then
# Check if TxWorkerAccounts exists under [State]
if grep -A 20 "^\[State\]" "$LIGHT_NODE_CONFIG_PATH" | grep -q "^[[:space:]]*TxWorkerAccounts"; then
# Check if TxWorkerAccounts exists anywhere in the config
if grep -q "^[[:space:]]*TxWorkerAccounts[[:space:]]*=" "$LIGHT_NODE_CONFIG_PATH"; then
# TxWorkerAccounts exists, check if it's set to 8
CURRENT_VALUE=$(grep -A 20 "^\[State\]" "$LIGHT_NODE_CONFIG_PATH" | grep "^[[:space:]]*TxWorkerAccounts" | head -1 | sed 's/.*=[[:space:]]*//')
CURRENT_VALUE=$(grep "^[[:space:]]*TxWorkerAccounts[[:space:]]*=" "$LIGHT_NODE_CONFIG_PATH" | head -1 | sed 's/.*=[[:space:]]*//')
if [ "$CURRENT_VALUE" != "8" ]; then
log "CONFIG" "Updating TxWorkerAccounts from $CURRENT_VALUE to 8"
# Update the value to 8 (only under [State] section)
if ! sed -i '/^\[State\]/,/^\[/ s/^[[:space:]]*TxWorkerAccounts[[:space:]]*=.*/ TxWorkerAccounts = 8/' "$LIGHT_NODE_CONFIG_PATH"; then
# Update the value to 8
if ! sed -i 's/^[[:space:]]*TxWorkerAccounts[[:space:]]*=.*/ TxWorkerAccounts = 8/' "$LIGHT_NODE_CONFIG_PATH"; then
Comment on lines +79 to +86
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The logic to update TxWorkerAccounts has been changed to operate on the entire configuration file, instead of being scoped to the [State] section. This is inconsistent with the logic that adds the key if it's missing (lines 95-102), which correctly places it under [State]. If TxWorkerAccounts were to exist in another section, this script could incorrectly modify it. The update logic should be scoped to the [State] section to maintain correctness and consistency. The previous implementation handled this correctly.

Suggested change
# Check if TxWorkerAccounts exists anywhere in the config
if grep -q "^[[:space:]]*TxWorkerAccounts[[:space:]]*=" "$LIGHT_NODE_CONFIG_PATH"; then
# TxWorkerAccounts exists, check if it's set to 8
CURRENT_VALUE=$(grep -A 20 "^\[State\]" "$LIGHT_NODE_CONFIG_PATH" | grep "^[[:space:]]*TxWorkerAccounts" | head -1 | sed 's/.*=[[:space:]]*//')
CURRENT_VALUE=$(grep "^[[:space:]]*TxWorkerAccounts[[:space:]]*=" "$LIGHT_NODE_CONFIG_PATH" | head -1 | sed 's/.*=[[:space:]]*//')
if [ "$CURRENT_VALUE" != "8" ]; then
log "CONFIG" "Updating TxWorkerAccounts from $CURRENT_VALUE to 8"
# Update the value to 8 (only under [State] section)
if ! sed -i '/^\[State\]/,/^\[/ s/^[[:space:]]*TxWorkerAccounts[[:space:]]*=.*/ TxWorkerAccounts = 8/' "$LIGHT_NODE_CONFIG_PATH"; then
# Update the value to 8
if ! sed -i 's/^[[:space:]]*TxWorkerAccounts[[:space:]]*=.*/ TxWorkerAccounts = 8/' "$LIGHT_NODE_CONFIG_PATH"; then
# Check if TxWorkerAccounts exists under [State]
if grep -A 20 "^\[State\]" "$LIGHT_NODE_CONFIG_PATH" | grep -q "^[[:space:]]*TxWorkerAccounts"; then
# TxWorkerAccounts exists, check if it's set to 8
CURRENT_VALUE=$(grep -A 20 "^\[State\]" "$LIGHT_NODE_CONFIG_PATH" | grep "^[[:space:]]*TxWorkerAccounts" | head -1 | sed 's/.*=[[:space:]]*//')
if [ "$CURRENT_VALUE" != "8" ]; then
log "CONFIG" "Updating TxWorkerAccounts from $CURRENT_VALUE to 8"
# Update the value to 8 (only under [State] section)
if ! sed -i '/^\[State\]/,/^\[/ s/^[[:space:]]*TxWorkerAccounts[[:space:]]*=.*/ TxWorkerAccounts = 8/' "$LIGHT_NODE_CONFIG_PATH"; then

log "ERROR" "Failed to update TxWorkerAccounts"
exit 1
fi
Expand Down
1 change: 1 addition & 0 deletions ev-stacks/stacks/single-sequencer/.env
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ SEQUENCER_EV_NODE_PROMETHEUS_PORT="26660"
SEQUENCER_DA_HEADER_NAMESPACE=
SEQUENCER_DA_DATA_NAMESPACE=
CHAIN_ID=
DA_SIGNING_ADDRESSES=
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ services:
- DA_ADDRESS=http://celestia-node:26658
- DA_HEADER_NAMESPACE=${SEQUENCER_DA_HEADER_NAMESPACE}
- DA_DATA_NAMESPACE=${SEQUENCER_DA_DATA_NAMESPACE}
- DA_SIGNING_ADDRESSES=${DA_SIGNING_ADDRESSES}
networks:
- evstack_shared

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ services:
- EVM_SIGNER_PASSPHRASE=${EVM_SIGNER_PASSPHRASE}
- DA_BLOCK_TIME=30s
- DA_ADDRESS=http://local-da:7980
- DA_SIGNING_ADDRESSES=${DA_SIGNING_ADDRESSES}
networks:
- evstack_shared

Expand Down
5 changes: 5 additions & 0 deletions ev-stacks/stacks/single-sequencer/entrypoint.sequencer.sh
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,11 @@ if [ -n "${DA_DATA_NAMESPACE:-}" ]; then
log "DEBUG" "Added DA data namespace flag: $DA_DATA_NAMESPACE"
fi

if [ -n "${DA_SIGNING_ADDRESSES:-}" ]; then
default_flags="$default_flags --rollkit.da.signing_addresses $DA_SIGNING_ADDRESSES"
log "DEBUG" "Added DA signing addresses flag: $DA_SIGNING_ADDRESSES"
fi
Comment on lines +189 to +192
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

This block of code, and similar blocks in this script, are vulnerable to command injection. The DA_SIGNING_ADDRESSES variable is concatenated into a command string without proper quoting or escaping, and later executed via eval on line 209. If DA_SIGNING_ADDRESSES contains shell metacharacters (e.g., ;, |, &, $(...)), it can lead to arbitrary command execution. This is a critical security risk.

The entire flag-building mechanism should be refactored to avoid using eval with concatenated strings. A safer approach is to properly escape each variable's value before adding it to the default_flags string.

Here is an example of how to safely add a variable:

if [ -n "${DA_SIGNING_ADDRESSES:-}" ]; then
	val_escaped=$(printf '%s' "$DA_SIGNING_ADDRESSES" | sed "s/'/'\\''/g")
	default_flags="$default_flags --rollkit.da.signing_addresses '$val_escaped'"
	log "DEBUG" "Added DA signing addresses flag: $DA_SIGNING_ADDRESSES"
fi

This pattern should be applied to all environment variables used to build flags in this script to mitigate the vulnerability.


default_flags="${default_flags} --home=${CONFIG_HOME}"

log "SUCCESS" "Configuration flags prepared successfully"
Expand Down