Skip to content

Commit fc3f2cb

Browse files
rfxnrfxn
committed
[Fix] PR #478 review: 3 bugs fixed, 4 false positives validated
[Fix] lmd_hook.sh: world-writable detection used broken glob extraction (${_perms##*[0-9][0-9]} always returned empty); replaced with ${_perms: -1} for correct last-octal-digit extraction; pr#478 [Fix] lmd_scan.sh: write resolved worker count to scan.meta instead of raw "auto" config value; prevents invalid JSON in -L --format json output; pr#478 [Fix] lmd_session.sh: --report list --format tsv now returns explicit error instead of silent text fallback; TSV remains supported for --report active and --report hooks; pr#478 [Change] tests: 5 new BATS tests (WW-01..WW-04 hook validation, TSV list rejection); updated TSV acceptance test to use --report active; 971 -> 976 tests
1 parent 5edb5fa commit fc3f2cb

7 files changed

Lines changed: 113 additions & 8 deletions

File tree

‎CHANGELOG‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,13 @@ v2.0.1 | Mar 25 2026:
124124

125125
-- Bug Fixes --
126126

127+
[Fix] Hook validation: world-writable detection used broken glob extraction that always
128+
returned empty string; replaced with bash substring to correctly extract last octal
129+
digit; pr#478
130+
[Fix] Scan lifecycle JSON: workers field emitted unquoted "auto" string producing invalid
131+
JSON; now resolves to integer before writing scan.meta; pr#478
132+
[Fix] Report list: --format tsv silently returned text output; now returns explicit error
133+
directing users to --report active for TSV output; pr#478
127134
[Fix] Background scans (-b): scan process recorded parent shell PID ($$) instead of
128135
actual forked PID (BASHPID); --kill/--pause/--stop targeted wrong process; scan.meta
129136
now stores ns_pid separately from pid for correct liveness/signal delivery

‎CHANGELOG.RELEASE‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,13 @@ v2.0.1 | Mar 25 2026:
124124

125125
-- Bug Fixes --
126126

127+
[Fix] Hook validation: world-writable detection used broken glob extraction that always
128+
returned empty string; replaced with bash substring to correctly extract last octal
129+
digit; pr#478
130+
[Fix] Scan lifecycle JSON: workers field emitted unquoted "auto" string producing invalid
131+
JSON; now resolves to integer before writing scan.meta; pr#478
132+
[Fix] Report list: --format tsv silently returned text output; now returns explicit error
133+
directing users to --report active for TSV output; pr#478
127134
[Fix] Background scans (-b): scan process recorded parent shell PID ($$) instead of
128135
actual forked PID (BASHPID); --kill/--pause/--stop targeted wrong process; scan.meta
129136
now stores ns_pid separately from pid for correct liveness/signal delivery

‎files/internals/lmd_hook.sh‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ _scan_hook_validate() {
8181
fi
8282

8383
# World-writable: last octal digit has bit 2 set (2, 3, 6, 7)
84-
_last_digit="${_perms##*[0-9][0-9]}"
84+
_last_digit="${_perms: -1}"
8585
case "$_last_digit" in
8686
2|3|6|7)
8787
eout "{hook} ERROR: post_scan_hook '$_hook_path' failed validation: world-writable"
@@ -101,7 +101,7 @@ _scan_hook_validate() {
101101
else
102102
_parent_perms=$("$stat" -c '%a' "$_parent" 2>/dev/null) || _parent_perms="" # stat may fail if parent was removed; safe to skip check
103103
fi
104-
local _parent_last="${_parent_perms##*[0-9][0-9]}"
104+
local _parent_last="${_parent_perms: -1}"
105105
case "$_parent_last" in
106106
2|3|6|7)
107107
eout "{hook} ERROR: post_scan_hook '$_hook_path' failed validation: parent directory is world-writable"

‎files/internals/lmd_scan.sh‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1070,8 +1070,15 @@ scan() {
10701070
if [ "${string_length_scan:-0}" == "1" ]; then
10711071
_scan_stages="${_scan_stages},strlen"
10721072
fi
1073+
# Resolve worker count to integer for meta (ClamAV/clamdscan = 1 worker)
1074+
local _meta_wk
1075+
if [ "$_engine_type" = "native" ]; then
1076+
_meta_wk=$(_resolve_worker_count "$tot_files")
1077+
else
1078+
_meta_wk=1
1079+
fi
10731080
_lifecycle_write_meta "$scanid" "$_scan_pid" "$PPID" "$hrspath" \
1074-
"$tot_files" "${scan_workers:-auto}" "$_engine_type" \
1081+
"$tot_files" "$_meta_wk" "$_engine_type" \
10751082
"$_effective_hashtype" "$_scan_stages" \
10761083
"scan_clamscan=$scan_clamscan,scan_yara=$scan_yara,quarantine_hits=$quarantine_hits"
10771084
fi

‎files/internals/lmd_session.sh‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -338,9 +338,9 @@ view_report() {
338338
fi
339339

340340
# --- TSV format restriction ---
341-
# TSV is only supported for list commands, not individual report rendering
342-
if [ "${_report_format:-}" = "tsv" ] && [ "$rid" != "list" ] && [ "$rid" != "active" ] && [ "$rid" != "hooks" ]; then
343-
echo "maldet($$): TSV format is only supported for list commands (--report list, --report active, -L)" >&2
341+
# TSV is only supported for active/hooks list commands, not report list or individual reports
342+
if [ "${_report_format:-}" = "tsv" ] && [ "$rid" != "active" ] && [ "$rid" != "hooks" ]; then
343+
echo "maldet($$): TSV format is only supported for active and hooks list commands (--report active, -L, --report hooks)" >&2
344344
return 1
345345
fi
346346

‎tests/39-lifecycle-list.bats‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -291,12 +291,17 @@ EOF
291291
# --format tsv validator
292292
# ========================================================================
293293

294-
@test "CLI: --format tsv is accepted without error" {
295-
run "$LMD_INSTALL/maldet" --format tsv --report list 2>&1
294+
@test "CLI: --format tsv is accepted for active list" {
295+
run "$LMD_INSTALL/maldet" --format tsv --report active 2>&1
296296
# Should NOT get "ERROR: --format requires text, json, or html"
297297
refute_output --partial "ERROR: --format requires"
298298
}
299299

300+
@test "CLI: --format tsv rejected for report list" {
301+
run "$LMD_INSTALL/maldet" --format tsv --report list 2>&1
302+
assert_output --partial "TSV format is only supported"
303+
}
304+
300305
@test "CLI: --format invalid is rejected" {
301306
run "$LMD_INSTALL/maldet" --format xml --report list 2>&1
302307
[ "$status" -ne 0 ]

‎tests/46-post-scan-hook.bats‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -560,3 +560,82 @@ TOHEOF
560560
run grep '"hook_timeout"' "$AUDIT_LOG"
561561
assert_success
562562
}
563+
564+
# ---------------------------------------------------------------------------
565+
# WW-01..WW-04: _scan_hook_validate world-writable checks (PR #478 Issue A)
566+
# ---------------------------------------------------------------------------
567+
568+
@test "WW-01: world-writable hook file (chmod 777) rejected" {
569+
local scan_dir
570+
scan_dir=$(mktemp -d)
571+
cp "$SAMPLES_DIR/eicar.com" "$scan_dir/"
572+
local ww_hook="$TEST_DIR/ww-hook.sh"
573+
cat > "$ww_hook" <<'WWEOF'
574+
#!/usr/bin/env bash
575+
echo "should not run"
576+
WWEOF
577+
chmod 777 "$ww_hook"
578+
lmd_set_config post_scan_hook "$ww_hook"
579+
lmd_set_config post_scan_hook_exec "sync"
580+
lmd_set_config post_scan_hook_min_hits "1"
581+
run maldet -co scan_hashtype=md5 -a "$scan_dir"
582+
rm -rf "$scan_dir"
583+
# Hook must NOT have run — marker absent
584+
[ ! -f "$HOOK_MARKER" ]
585+
# Scan still completes (validation failure is not a scan error)
586+
[ "$status" -ne 1 ]
587+
}
588+
589+
@test "WW-02: world-writable parent directory rejected" {
590+
local ww_parent
591+
ww_parent=$(mktemp -d)
592+
chmod 1777 "$ww_parent"
593+
local ww_hook="$ww_parent/hook.sh"
594+
cat > "$ww_hook" <<'WWEOF'
595+
#!/usr/bin/env bash
596+
echo "should not run"
597+
WWEOF
598+
chmod 755 "$ww_hook"
599+
local scan_dir
600+
scan_dir=$(mktemp -d)
601+
cp "$SAMPLES_DIR/eicar.com" "$scan_dir/"
602+
lmd_set_config post_scan_hook "$ww_hook"
603+
lmd_set_config post_scan_hook_exec "sync"
604+
lmd_set_config post_scan_hook_min_hits "1"
605+
run maldet -co scan_hashtype=md5 -a "$scan_dir"
606+
rm -rf "$scan_dir" "$ww_parent"
607+
[ ! -f "$HOOK_MARKER" ]
608+
[ "$status" -ne 1 ]
609+
}
610+
611+
@test "WW-03: non-world-writable hook (chmod 755) accepted" {
612+
local scan_dir
613+
scan_dir=$(mktemp -d)
614+
cp "$SAMPLES_DIR/eicar.com" "$scan_dir/"
615+
# HOOK_SCRIPT is already 755 from setup()
616+
lmd_set_config post_scan_hook "$HOOK_SCRIPT"
617+
lmd_set_config post_scan_hook_exec "sync"
618+
lmd_set_config post_scan_hook_format "args"
619+
lmd_set_config post_scan_hook_min_hits "1"
620+
run maldet -co scan_hashtype=md5 -a "$scan_dir"
621+
rm -rf "$scan_dir"
622+
# Hook should have fired — marker present
623+
[ -f "$HOOK_MARKER" ]
624+
}
625+
626+
@test "WW-04: setuid mode (chmod 4755) accepted (not world-writable)" {
627+
local scan_dir
628+
scan_dir=$(mktemp -d)
629+
cp "$SAMPLES_DIR/eicar.com" "$scan_dir/"
630+
# Reuse HOOK_SCRIPT from setup() (writes to HOOK_MARKER), but set 4755
631+
chmod 4755 "$HOOK_SCRIPT"
632+
lmd_set_config post_scan_hook "$HOOK_SCRIPT"
633+
lmd_set_config post_scan_hook_exec "sync"
634+
lmd_set_config post_scan_hook_format "args"
635+
lmd_set_config post_scan_hook_min_hits "1"
636+
run maldet -co scan_hashtype=md5 -a "$scan_dir"
637+
rm -rf "$scan_dir"
638+
# 4755 is not world-writable -- hook should pass validation and fire
639+
[ "$status" -ne 1 ]
640+
[ -f "$HOOK_MARKER" ]
641+
}

0 commit comments

Comments
 (0)