Skip to content

Commit aaa1ed9

Browse files
underhaxunderhax
committed
Enhance security validation and optimize utilities
- Add SERVER_NAME validation to prevent HTTP header injections - URL-encode Matrix room_id and upload filename per RFC 3986 - Replace bc with awk for float comparison to reduce dependencies - Replace echo with printf in dispatch error logs for consistency - Introduce urlencode utility in common library - Update comprehensive BATS test suites for all modifications
1 parent 2d9ec7f commit aaa1ed9

9 files changed

Lines changed: 92 additions & 27 deletions

File tree

‎README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ The login monitor focuses on immediate visibility into system access. It monitor
4646
### 2. Dependencies
4747
Install the necessary utilities using the package manager:
4848
```bash
49-
sudo apt update && sudo apt install -y awk bc curl jq procps coreutils file
49+
sudo apt update && sudo apt install -y awk curl jq procps coreutils file
5050
```
5151

5252
## Installation

‎opt/monitoring/high-load.sh‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,7 @@ collect_top_procs() {
255255

256256
should_alert() {
257257
local load_exceeded
258-
load_exceeded=$(echo "${LOAD_1} > ${THRESHOLD}" | bc -l)
258+
load_exceeded=$(awk -v l="${LOAD_1}" -v t="${THRESHOLD}" 'BEGIN {print (l > t) ? 1 : 0}')
259259
[[ ${load_exceeded} -eq 1 ]] || return 1
260260

261261
if [[ ${PSI_AVAILABLE} -eq 1 ]]; then
@@ -353,27 +353,27 @@ dispatch_notifications() {
353353
telegram)
354354
local message
355355
message=$(high_load_message_telegram)
356-
tg_send_message "${message}" || echo "ERROR: telegram notification failed" >&2
357-
tg_send_file "${TOP_PROCS_FILE}" || echo "ERROR: telegram file send failed" >&2
356+
tg_send_message "${message}" || printf "ERROR: telegram notification failed\n" >&2
357+
tg_send_file "${TOP_PROCS_FILE}" || printf "ERROR: telegram file send failed\n" >&2
358358
;;
359359
matrix)
360360
local plain html
361361
plain=$(high_load_message_matrix_plain)
362362
html=$(high_load_message_matrix_html)
363-
mx_send_message "${plain}" "${html}" || echo "ERROR: matrix notification failed" >&2
364-
mx_send_file "${TOP_PROCS_FILE}" || echo "ERROR: matrix file send failed" >&2
363+
mx_send_message "${plain}" "${html}" || printf "ERROR: matrix notification failed\n" >&2
364+
mx_send_file "${TOP_PROCS_FILE}" || printf "ERROR: matrix file send failed\n" >&2
365365
;;
366366
ntfy)
367367
local message title
368368
message=$(high_load_message_ntfy)
369369
title=$(high_load_title_ntfy)
370370
# shellcheck disable=SC2154
371371
# Rationale: NTFY_TOKEN is populated from config file.
372-
ntfy_send "${message}" "${NTFY_URL}" "${NTFY_TOPIC}" "${NTFY_TOKEN}" "${title}" || echo "ERROR: ntfy notification failed" >&2
373-
ntfy_send_file "${TOP_PROCS_FILE}" "${NTFY_URL}" "${NTFY_TOPIC}" "${NTFY_TOKEN}" "${title}" || echo "ERROR: ntfy file send failed" >&2
372+
ntfy_send "${message}" "${NTFY_URL}" "${NTFY_TOPIC}" "${NTFY_TOKEN}" "${title}" || printf "ERROR: ntfy notification failed\n" >&2
373+
ntfy_send_file "${TOP_PROCS_FILE}" "${NTFY_URL}" "${NTFY_TOPIC}" "${NTFY_TOKEN}" "${title}" || printf "ERROR: ntfy file send failed\n" >&2
374374
;;
375375
*)
376-
echo "ERROR: Unknown notifier: ${notifier}" >&2
376+
printf "ERROR: Unknown notifier: %s\n" "${notifier}" >&2
377377
;;
378378
esac
379379
) &
@@ -384,7 +384,7 @@ dispatch_notifications() {
384384

385385
main() {
386386
parse_args "$@"
387-
check_deps awk bc curl jq ps file stat
387+
check_deps awk curl jq ps file stat
388388

389389
if [[ -z ${SERVER_NAME:-} ]] && [[ -f ${CONFIG_CACHE} ]]; then
390390
# shellcheck source=/dev/null

‎opt/monitoring/lib/common.sh‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,23 @@ check_deps() {
2121
[[ ${#missing[@]} -eq 0 ]] || die "Missing required commands: ${missing[*]}"
2222
}
2323

24+
urlencode() {
25+
local string="${1}"
26+
local strlen=${#string}
27+
local encoded=""
28+
local pos c o
29+
30+
for ((pos = 0; pos < strlen; pos++)); do
31+
c=${string:pos:1}
32+
case "${c}" in
33+
[-_.~a-zA-Z0-9]) o="${c}" ;;
34+
*) printf -v o '%%%02x' "'${c}" ;;
35+
esac
36+
encoded+="${o}"
37+
done
38+
printf '%s' "${encoded}"
39+
}
40+
2441
load_senders() {
2542
# shellcheck disable=SC2154
2643
# Rationale: SENDERS_DIR is defined in the main script and imported via source

‎opt/monitoring/lib/validation.sh‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,9 @@ validate_bot_config() {
2424
# Rationale: Configuration file path is resolved dynamically at runtime.
2525
source "${config_file}"
2626

27-
[[ -n ${SERVER_NAME:-} ]] || {
28-
printf "SERVER_NAME is not set\n" >&2
29-
return 1
30-
}
27+
# shellcheck disable=SC2310
28+
# Rationale: Function intentionally returns error code for parent script to handle.
29+
validate_server_name "${SERVER_NAME:-}" || return 1
3130

3231
local has_telegram=0
3332
local has_matrix=0
@@ -350,6 +349,19 @@ validate_matrix_room_id() {
350349
return 0
351350
}
352351

352+
validate_server_name() {
353+
local val="$1"
354+
[[ -n ${val} ]] || {
355+
printf "SERVER_NAME is not set\n" >&2
356+
return 1
357+
}
358+
[[ ${val} =~ ^[a-zA-Z0-9\ ._-]+$ ]] || {
359+
printf "SERVER_NAME format is invalid (allowed: a-z, A-Z, 0-9, space, ., _, -)\n" >&2
360+
return 1
361+
}
362+
return 0
363+
}
364+
353365
validate_bot_token() {
354366
local val="$1"
355367
[[ -n ${val} ]] || {

‎opt/monitoring/login.sh‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -121,24 +121,24 @@ dispatch_login_notification() {
121121
telegram)
122122
local message
123123
message=$(login_message_telegram)
124-
tg_send_message "${message}" || echo "ERROR: telegram notification failed" >&2
124+
tg_send_message "${message}" || printf "ERROR: telegram notification failed\n" >&2
125125
;;
126126
matrix)
127127
local plain html
128128
plain=$(login_message_matrix_plain)
129129
html=$(login_message_matrix_html)
130-
mx_send_message "${plain}" "${html}" || echo "ERROR: matrix notification failed" >&2
130+
mx_send_message "${plain}" "${html}" || printf "ERROR: matrix notification failed\n" >&2
131131
;;
132132
ntfy)
133133
local message title
134134
message=$(login_message_ntfy)
135135
title=$(login_title_ntfy)
136136
# shellcheck disable=SC2154
137137
# Rationale: NTFY_* variables are populated from config file at runtime
138-
ntfy_send "${message}" "${NTFY_URL}" "${NTFY_TOPIC}" "${NTFY_TOKEN}" "${title}" || echo "ERROR: ntfy notification failed" >&2
138+
ntfy_send "${message}" "${NTFY_URL}" "${NTFY_TOPIC}" "${NTFY_TOKEN}" "${title}" || printf "ERROR: ntfy notification failed\n" >&2
139139
;;
140140
*)
141-
echo "ERROR: Unknown notifier: ${notifier}" >&2
141+
printf "ERROR: Unknown notifier: %s\n" "${notifier}" >&2
142142
;;
143143
esac
144144
) &

‎opt/monitoring/senders/matrix.sh‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,9 @@ mx_send_message() {
1616
local txn_id
1717
txn_id=$(mx_txn_id)
1818

19-
local endpoint="${url}/_matrix/client/v3/rooms/${room_id}/send/m.room.message/${txn_id}"
19+
local encoded_room
20+
encoded_room=$(urlencode "${room_id}")
21+
local endpoint="${url}/_matrix/client/v3/rooms/${encoded_room}/send/m.room.message/${txn_id}"
2022

2123
local payload
2224
payload=$(jq -n \
@@ -63,8 +65,9 @@ mx_upload_file() {
6365
local mime_type
6466
mime_type=$(file -b --mime-type "${file_path}")
6567

66-
local filename
68+
local filename encoded_filename
6769
filename=$(basename "${file_path}")
70+
encoded_filename=$(urlencode "${filename}")
6871

6972
local upload_response content_uri
7073
upload_response=$(curl -fsSL \
@@ -76,7 +79,7 @@ mx_upload_file() {
7679
-H "Authorization: Bearer ${access_token}" \
7780
-H "Content-Type: ${mime_type}" \
7881
--data-binary "@${file_path}" \
79-
"${url}/_matrix/media/v3/upload?filename=${filename}") || {
82+
"${url}/_matrix/media/v3/upload?filename=${encoded_filename}") || {
8083
printf "ERROR [matrix]: File upload curl failed\n" >&2
8184
return 1
8285
}
@@ -104,7 +107,9 @@ mx_send_file() {
104107
filename=$(basename "${file_path}")
105108
txn_id=$(mx_txn_id)
106109

107-
local endpoint="${url}/_matrix/client/v3/rooms/${room_id}/send/m.room.message/${txn_id}"
110+
local encoded_room
111+
encoded_room=$(urlencode "${room_id}")
112+
local endpoint="${url}/_matrix/client/v3/rooms/${encoded_room}/send/m.room.message/${txn_id}"
108113

109114
local payload
110115
payload=$(jq -n \

‎tests/high-load.bats‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ setup() {
172172
if [[ "$1" == "-v" ]]; then return 0; fi
173173
builtin command "$@"
174174
}
175-
run check_deps awk bc curl jq ps file stat
175+
run check_deps awk curl jq ps file stat
176176
[ "$status" -eq 1 ]
177177
[[ "$output" == *"Missing required commands: awk"* ]]
178178
}
@@ -278,19 +278,19 @@ setup() {
278278
[ -z "$FAILED_SERVICES" ]
279279
}
280280

281-
@test "main calls check_deps with awk bc curl jq ps file stat" {
281+
@test "main calls check_deps with awk curl jq ps file stat" {
282282
check_deps() { printf "DEPS_CALLED:%s\n" "$*"; }
283283

284284
run main --threshold 5.0
285-
[[ "$output" == *"DEPS_CALLED:awk bc curl jq ps file stat"* ]]
285+
[[ "$output" == *"DEPS_CALLED:awk curl jq ps file stat"* ]]
286286
}
287287

288288
@test "main fails when check_deps reports missing commands" {
289-
check_deps() { die "Missing required commands: bc"; }
289+
check_deps() { die "Missing required commands: jq"; }
290290

291291
run main --threshold 5.0
292292
[ "$status" -eq 1 ]
293-
[[ "$output" == *"Missing required commands: bc"* ]]
293+
[[ "$output" == *"Missing required commands: jq"* ]]
294294
}
295295

296296
@test "main fails without --threshold" {

‎tests/senders.bats‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ teardown_file() {
1414
setup() {
1515
export SCRIPT_DIR="${BATS_TEST_DIRNAME}/../opt/monitoring"
1616
export SENDERS_DIR="${SCRIPT_DIR}/senders"
17+
source "${SCRIPT_DIR}/lib/common.sh"
1718
source "${SENDERS_DIR}/telegram.sh"
1819
source "${SENDERS_DIR}/matrix.sh"
1920
source "${SENDERS_DIR}/ntfy.sh"

‎tests/validation.bats‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -487,6 +487,36 @@ teardown_file() {
487487
[ "$status" -eq 1 ]
488488
}
489489

490+
@test "validate_server_name accepts valid names" {
491+
run validate_server_name "my-server-01"
492+
[ "$status" -eq 0 ]
493+
run validate_server_name "prod.web.us-east"
494+
[ "$status" -eq 0 ]
495+
run validate_server_name "Server_Name.123"
496+
[ "$status" -eq 0 ]
497+
run validate_server_name "server name"
498+
[ "$status" -eq 0 ]
499+
}
500+
501+
@test "validate_server_name rejects empty name" {
502+
run validate_server_name ""
503+
[ "$status" -eq 1 ]
504+
[[ "$output" == *"SERVER_NAME is not set"* ]]
505+
}
506+
507+
@test "validate_server_name rejects CRLF injection" {
508+
run validate_server_name $'server\r\nX-Injected: true'
509+
[ "$status" -eq 1 ]
510+
[[ "$output" == *"SERVER_NAME format is invalid"* ]]
511+
}
512+
513+
@test "validate_server_name rejects special chars" {
514+
run validate_server_name "server/../../etc"
515+
[ "$status" -eq 1 ]
516+
run validate_server_name 'server$(whoami)'
517+
[ "$status" -eq 1 ]
518+
}
519+
490520
@test "validate_ntfy_topic accepts valid topic" {
491521
run validate_ntfy_topic "my-topic"
492522
[ "$status" -eq 0 ]

0 commit comments

Comments
 (0)