From c98d757c0fadc66f00dd07b49f8e8d14fb5a774d Mon Sep 17 00:00:00 2001 From: Luke Parker Date: Tue, 9 Dec 2025 04:03:24 -0500 Subject: [PATCH] Ensure the signed arithmetic won't overflow, expand shells tested with (#701) * Normalize naming for the stack size CI file * Extend stack size CI with `posh` and `lksh` `posh` is a derivative of `pdksh` explicitly intended for ensuring Debian-policy-compliance. `lksh` is more-POSIX-esque, legacy shell included along-side `mksh`. * Ensure a signed long overflow won't occur Also fixes `write_bytes` when the written bytes have alphabetical digits when encoded as hexadecimal. * Improve sh semantics in stack-size workflow --- .../{stack_size.yml => stack-size.yml} | 16 ++++++------- orchestration/increase_default_stack_size.sh | 23 +++++++++++++------ 2 files changed, 24 insertions(+), 15 deletions(-) rename .github/workflows/{stack_size.yml => stack-size.yml} (83%) diff --git a/.github/workflows/stack_size.yml b/.github/workflows/stack-size.yml similarity index 83% rename from .github/workflows/stack_size.yml rename to .github/workflows/stack-size.yml index 031d3f63..83c9b9c9 100644 --- a/.github/workflows/stack_size.yml +++ b/.github/workflows/stack-size.yml @@ -37,21 +37,21 @@ jobs: cd chelf git checkout b2994186cea7b7d61a588fd06c1cc1ae75bcc21a make - ./chelf -s $STACK ../monerod-chelf + ./chelf -s "$STACK" ../monerod-chelf cd .. cp monerod monerod-muslstack GOBIN=$(pwd) go install github.com/yaegashi/muslstack@d19cc5866abce3ca59dfc1666df7cc97097d0933 - ./muslstack -s $STACK ./monerod-muslstack + ./muslstack -s "$STACK" ./monerod-muslstack sudo apt update -y - sudo apt install -y ksh bash dash zsh busybox mksh yash - sudo ln -s $(which busybox) /usr/bin/ash - sudo ln -s $(which busybox) /usr/bin/hush + sudo apt install -y ksh bash dash zsh busybox mksh posh yash + sudo ln -s "$(which busybox)" /usr/bin/ash + sudo ln -s "$(which busybox)" /usr/bin/hush cargo install brush-shell - for shell in sh ksh bash dash zsh ash hush mksh yash brush; do + for shell in sh ksh bash dash zsh ash hush mksh lksh posh yash brush; do cp monerod monerod-idss-$shell - ln -s $(which $shell) sh + ln -s "$(which $shell)" sh ./sh ./orchestration/increase_default_stack_size.sh monerod-idss-$shell rm ./sh done @@ -70,7 +70,7 @@ jobs: read_stack() { STACK_INFO=$(readelf "$1" -l | grep STACK -A1) - MEMSZ=$(echo "$STACK_INFO" | tail -n1 | sed -E s/^[[:space:]]*//g | cut -f2 -d' ') + MEMSZ=$(printf "%s\n" "$STACK_INFO" | tail -n1 | sed -E s/^[[:space:]]*//g | cut -f2 -d' ') printf "%i" $((MEMSZ)) } INITIAL_STACK=$(read_stack monerod) diff --git a/orchestration/increase_default_stack_size.sh b/orchestration/increase_default_stack_size.sh index b989808c..ead24fdc 100755 --- a/orchestration/increase_default_stack_size.sh +++ b/orchestration/increase_default_stack_size.sh @@ -27,7 +27,8 @@ read_bytes() { dd bs=1 skip="$1" count="$2" if="$ELF" 2> /dev/null | hex } hex_to_octal() { - printf "ibase=16; obase=8; %s\n" "$1" | bc + HEX=$(printf "%s" "$1" | tr "[:lower:]" "[:upper:]") + printf "ibase=16; obase=8; %s\n" "$HEX" | bc } write_bytes() { POS=$1 @@ -47,7 +48,7 @@ write_bytes() { # Magic MAGIC=$(read_bytes 0 4) # shellcheck disable=SC2059 -EXPECTED_MAGIC=$(printf \\"$(hex_to_octal 7F)"ELF | hex) +EXPECTED_MAGIC=$(printf \\"$(hex_to_octal 7f)"ELF | hex) if [ ! "$MAGIC" = "$EXPECTED_MAGIC" ]; then echo "Not ELF" exit 2 @@ -78,6 +79,12 @@ read_integer_by_offset() { OFFSET=$(value_per_bits "$1" "$2") BYTES=$(read_bytes "$OFFSET" "$3") BYTES=$(swap_native_endian "$BYTES") + BYTES=$(printf "%s" "$BYTES" | tr "[:lower:]" "[:upper:]") + LESS_THAN_SANITY=$(printf "ibase=16; if(%s < 6FFFFFFF)1;\n" "$BYTES" | bc) + if [ ! "$LESS_THAN_SANITY" = "1" ]; then + echo "Integer value is approximate to 2**31, risking a signed long overflow" + exit 4 + fi printf "%i" $(( 0x$BYTES )) } @@ -88,7 +95,7 @@ case $LITTLE_ENDIAN in "02") LITTLE_ENDIAN=0;; *) echo "Not little- or big- endian" - exit 4 + exit 5 ;; esac @@ -119,13 +126,13 @@ swap_native_endian() { ELF_VERSION=$(read_bytes 6 1) if [ ! "$ELF_VERSION" = "01" ]; then echo "Unknown ELF Version ($ELF_VERSION)" - exit 5 + exit 6 fi ELF_VERSION_2=$(read_bytes $((0x14)) 4) if [ ! "$ELF_VERSION_2" = "$(swap_native_endian 00000001)" ]; then echo "Unknown secondary ELF Version ($ELF_VERSION_2)" - exit 6 + exit 7 fi # Find where the program headers are @@ -134,7 +141,7 @@ PROGRAM_HEADER_SIZE=$(value_per_bits 0x20 0x38) DECLARED_PROGRAM_HEADER_SIZE=$(read_integer_by_offset 0x2a 0x36 2) if [ ! "$PROGRAM_HEADER_SIZE" -eq "$DECLARED_PROGRAM_HEADER_SIZE" ]; then echo "Unexpected size of a program header ($DECLARED_PROGRAM_HEADER_SIZE)" - exit 7 + exit 8 fi program_header_start() { printf "%i" $((PROGRAM_HEADERS_OFFSET + ($1 * PROGRAM_HEADER_SIZE))) @@ -162,6 +169,8 @@ while [ "$NEXT_PROGRAM_HEADER" -ne -1 ]; do fi FOUND=1 + # This line is the only line really risking an arithmetic overflow, yet the bound on the start of + # the section, combined with a maximum section length of `0xffff * 0x38`, makes this fit MEMSZ_OFFSET=$(( $(program_header_start "$THIS_PROGRAM_HEADER") + $(value_per_bits 0x14 0x28) )) MEMSZ_LEN=$(value_per_bits 4 8) # `MEMSZ_OFFSET MEMSZ_OFFSET` as we've already derived it depending on the amount of bits @@ -178,7 +187,7 @@ done if [ "$FOUND" -eq 0 ]; then echo "\`PT_GNU_STACK\` program header not found" - exit 8 + exit 9 fi echo "All instances of \`PT_GNU_STACK\` patched to be at least 8 MB"