Skip to content

Fix crash in execve filename parameter parsing #98

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Stringy wants to merge 1 commit into
base: 0.23.1-stackrox
Choose a base branch
from
Open

Fix crash in execve filename parameter parsing #98

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
18 changes: 9 additions & 9 deletions userspace/libsinsp/event.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,11 +213,14 @@ inline std::string_view get_event_param_as<std::string_view>(const sinsp_evt_par
}

size_t string_len = strnlen(param_data, param_len);
// We expect the parameter to be exactly one null-terminated string
// We expect the parameter to be exactly one null-terminated string.
// When it doesn't match, use the full parameter length instead.
// Event parameter data can have a recorded length that doesn't match
// the null-terminated string length. This has been observed on some
// kernels for both string and integer parameters (e.g. clone3 exe
// param_len=5 vs strnlen=1, clone flags param_len=597 vs expected 4).
if(param_len != string_len + 1) {
// By moving this error string building operation to a separate function
// the compiler is more likely to inline this entire function.
param.throw_invalid_len_error(string_len + 1);
string_len = param_len - 1;
Comment on lines +216 to +223

@coderabbitai coderabbitai Bot Aug 5, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Update invalid_string_len for the new contract.

userspace/libsinsp/test/events_param.ut.cpp Lines 515-539 corrupt an embedded NUL and expect advance_ts_get_event to throw. This branch now accepts that mismatch, so the test still asserts the removed exception path. Change it to assert successful processing. Add a separate test for missing terminators or zero recorded lengths if those inputs remain invalid.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@userspace/libsinsp/event.h` around lines 216 - 223, Update the tests around
invalid_string_len and advance_ts_get_event so an embedded-NUL length mismatch
is accepted and processed successfully instead of expecting an exception.
Preserve coverage for inputs that remain invalid by adding a separate test for a
missing terminator or zero recorded length, using the existing event-parameter
test helpers and assertions.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n -C 10 'strnlen|param_len|get_event_param_as' userspace/libsinsp/event.h
rg -n -C 8 'as<std::string_view>|snprintf.*%s' userspace/libsinsp/event.cpp

Repository: stackrox/falcosecurity-libs

Length of output: 9343

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- event.h relevant declarations and implementation ---'
ast-grep outline userspace/libsinsp/event.h
sed -n '70,185p' userspace/libsinsp/event.h
sed -n '189,243p' userspace/libsinsp/event.h

echo '--- data_and_len_with_legacy_null_encoding definitions/usages ---'
rg -n -C 12 'data_and_len_with_legacy_null_encoding|sinsp_evt_param::sinsp_evt_param|m_data|m_len' userspace/libsinsp userspace/libscap test 2>/dev/null | head -n 320

echo '--- formatter callers and parameter types ---'
sed -n '820,900p' userspace/libsinsp/event.cpp
rg -n -C 8 'PT_CHARBUF|as<std::string_view>|as<std::string>' userspace/libsinsp userspace/libscap test 2>/dev/null | head -n 360

echo '--- tests around mismatched lengths ---'
sed -n '470,560p' userspace/libsinsp/test/events_param.ut.cpp

Repository: stackrox/falcosecurity-libs

Length of output: 50383

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- event parameter construction and loading ---'
rg -n -C 18 'sinsp_evt_param\(|m_params\.|load_params|get_param\(' userspace/libsinsp/event.h userspace/libsinsp/event.cpp userspace/libsinsp/*.cpp | head -n 420

echo '--- raw event/scap length definitions and parser code ---'
rg -n -C 16 'PPM_EVENT_HEADER|len.*param|param.*len|scap_evt|nparams|parse.*param|next.*param' userspace/libsinsp userspace/libscap | head -n 500

echo '--- tests for string conversion and malformed lengths ---'
rg -n -C 12 'strnlen|std::string_view|mismatched|param_len|invalid_len|as<std::string' userspace/libsinsp/test test 2>/dev/null | head -n 500

echo '--- standalone bounded-scan and formatter probe ---'
python3 - <<'PY'
import ctypes
libc = ctypes.CDLL(None)
libc.strnlen.argtypes = [ctypes.c_char_p, ctypes.c_size_t]
libc.strnlen.restype = ctypes.c_size_t

cases = [
    (b"abc\0", 4),
    (b"abc", 3),
    (b"abc\0XYZ", 7),
    (b"\0", 1),
]
for data, length in cases:
    n = libc.strnlen(data, length)
    print({"data": data, "param_len": length, "strnlen": n,
           "fallback_len": length - 1 if length != n + 1 else n,
           "has_nul": n < length})
PY

cat >/tmp/probe_percent_s.c <<'C'
`#include` <stdio.h>
`#include` <string.h>
int main(void) {
    char data[] = {'a','b','c','X','Y','Z','\0'};
    char out[32];
    int n = snprintf(out, sizeof out, "%s", data);
    printf("formatted=%s length=%zu count=%d\n", out, strlen(out), n);
    return 0;
}
C
cc /tmp/probe_percent_s.c -o /tmp/probe_percent_s
/tmp/probe_percent_s

Repository: stackrox/falcosecurity-libs

Length of output: 50384

🏁 Script executed:

#!/bin/bash
set -u

echo '--- event.h event loading ---'
sed -n '560,635p' userspace/libsinsp/event.h

echo '--- event.cpp parameter APIs ---'
rg -n -C 20 'get_param_as_str|get_param\(|load_params|sinsp_evt_param\(' userspace/libsinsp/event.cpp userspace/libsinsp/event.h || true

echo '--- parameter parser symbols ---'
rg -n -C 12 'm_params|ppm_param_info|param_info|nparams|event.*param|param.*event' userspace/libsinsp/event.cpp userspace/libsinsp/event.h | head -n 420 || true

echo '--- exact parameter tests ---'
rg -n -C 18 'throw|invalid|mismatch|param_len|strnlen|as<std::string_view>|as<std::string>' userspace/libsinsp/test/events_param.ut.cpp || true

echo '--- capture format and event length declarations ---'
rg -n -C 10 'struct scap_evt|scap_evt.*len|len.*scap_evt|PPM_EVENT_HEADER|ppm_evt_hdr' userspace/libscap userspace/libsinsp | head -n 320 || true

echo '--- standalone bounded-scan probe ---'
python3 - <<'PY'
import ctypes
libc = ctypes.CDLL(None)
libc.strnlen.argtypes = [ctypes.c_char_p, ctypes.c_size_t]
libc.strnlen.restype = ctypes.c_size_t
for data, length in [(b"abc\0", 4), (b"abc", 3), (b"abc\0XYZ", 7), (b"\0", 1)]:
    n = libc.strnlen(data, length)
    print(f"data={data!r} param_len={length} strnlen={n} "
          f"has_nul={n < length} fallback_len={length - 1 if length != n + 1 else n}")
PY

cat >/tmp/probe_percent_s.c <<'C'
`#include` <stdio.h>
`#include` <string.h>
int main(void) {
    char data[] = {'a','b','c','X','Y','Z','\0'};
    char out[32];
    int n = snprintf(out, sizeof out, "%s", data);
    printf("formatted=%s length=%zu count=%d\n", out, strlen(out), n);
    return 0;
}
C
cc /tmp/probe_percent_s.c -o /tmp/probe_percent_s
/tmp/probe_percent_s

Repository: stackrox/falcosecurity-libs

Length of output: 50385

🏁 Script executed:

#!/bin/bash
set -u

echo '--- decoder locations ---'
rg -l 'scap_event_decode_params' . | head -n 80

echo '--- decoder implementation and declarations ---'
for f in $(rg -l 'scap_event_decode_params' userspace); do
  echo "### $f"
  rg -n -C 35 'scap_event_decode_params' "$f" || true
done

echo '--- focused tests ---'
sed -n '480,555p' userspace/libsinsp/test/events_param.ut.cpp
rg -n -C 14 'std::string_view|as<std::string>|param_len|strnlen|throw_invalid_len' userspace/libsinsp/test/events_param.ut.cpp || true

echo '--- string parameter metadata ---'
rg -n -C 8 'PT_CHARBUF|PT_FSPATH|PT_FSRELPATH' driver userspace/libscap userspace/libsinsp | head -n 360 || true

echo '--- changed-file diff summary and nearby test changes ---'
git diff --stat -- userspace/libsinsp/event.h userspace/libsinsp/test/events_param.ut.cpp
git diff -- userspace/libsinsp/event.h userspace/libsinsp/test/events_param.ut.cpp | sed -n '1,260p'

Repository: stackrox/falcosecurity-libs

Length of output: 41125

🏁 Script executed:

#!/bin/bash
set -u

echo '--- driver string length and ring encoding ---'
sed -n '450,555p' driver/ppm_events.c
rg -n -C 18 'ppm_strncpy_from_user|strnlen\(args->str_storage|val_to_ring\(args' driver/ppm_fillers.c driver/ppm_events.c | head -n 360

echo '--- event buffer ownership and allocation boundaries ---'
rg -n -C 16 'scap_evt.*len|m_pevt_storage|from_scap_evt|event_size|malloc.*evt|realloc.*evt' userspace/libscap userspace/libsinsp | head -n 420

echo '--- PT_CHARBUF formatter context ---'
sed -n '697,890p' userspace/libsinsp/event.cpp

echo '--- test event construction helper declarations ---'
rg -n -C 16 'add_event\(|add_event_advance_ts|scap_const_sized_buffer|scap_event_encode' userspace/libsinsp/test/events_param.ut.cpp test/libscap/test_suites/userspace/scap_event.cpp | head -n 360

Repository: stackrox/falcosecurity-libs

Length of output: 50383

🏁 Script executed:

#!/bin/bash
set -u

echo '--- malformed-length test call path ---'
rg -n -C 24 'advance_ts_get_event|invalid_string_len|EXPECT_THROW' userspace/libsinsp/test userspace/libsinsp | head -n 420 || true

echo '--- all direct PT_CHARBUF string formatting paths ---'
rg -n -C 8 'as<std::string_view>|"%s".*s\.data|snprintf.*s\.data' userspace/libsinsp userspace/libscap | head -n 300 || true

echo '--- decoder boundary and sentinel definitions ---'
sed -n '88,132p' userspace/libscap/scap_event.c
rg -n -C 10 'PPM_ENABLE_SENTINEL|sentinel_begin|sentinel_end|struct ppm_evt_hdr' driver/ppm_events_public.h userspace/libscap | head -n 320 || true

Repository: stackrox/falcosecurity-libs

Length of output: 50383

Make PT_CHARBUF formatting length-aware.

Malformed or capture-file input can produce strnlen(param_data, param_len) == param_len. The fallback then returns a std::string_view without a NUL, but userspace/libsinsp/event.cpp:877 passes s.data() to %s. The destination bound does not limit source reads. Use a length-aware formatter or reject the no-NUL case before formatting.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@userspace/libsinsp/event.h` around lines 216 - 223, The PT_CHARBUF formatting
path around the parameter-length fallback must not pass a non-NUL-terminated
buffer via s.data() to %s. Update the formatting logic used by event.cpp to use
a length-aware formatter with the string view’s size, or reject the no-NUL case
before formatting, while preserving normal NUL-terminated string handling.

}

return {param_data, string_len};
Expand All @@ -231,14 +234,11 @@ inline std::string get_event_param_as<std::string>(const sinsp_evt_param& param)
}

size_t string_len = strnlen(param_data, param_len);
// We expect the parameter to be exactly one null-terminated string
if(param_len != string_len + 1) {
// By moving this error string building operation to a separate function
// the compiler is more likely to inline this entire function.
param.throw_invalid_len_error(string_len + 1);
string_len = param_len - 1;
}

return std::string(param_data);
return std::string(param_data, string_len);
}

template<>
Expand Down