Skip to content

XCCDF_POLICY: bound recursion on cyclic Profile @extends #2364

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
edznux-dd wants to merge 1 commit into
base: main
Choose a base branch
from
+22 −1
Open

XCCDF_POLICY: bound recursion on cyclic Profile @extends #2364

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
23 changes: 22 additions & 1 deletion src/XCCDF_POLICY/xccdf_policy.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1864,15 +1864,34 @@ xccdf_policy_add_select(struct xccdf_policy *policy, struct xccdf_select *sel)
return _xccdf_policy_add_selector_internal(policy, benchmark, sel, true);
}

// Upper bound on the Profile @extends chain length. Real content nests at most
// a handful deep; this only exists to stop a cyclic @extends chain from
// recursing until the stack overflows.
#define XCCDF_POLICY_MAX_EXTENDS_DEPTH 64

// Nesting counter for profile @extends resolution. It is a file-static (rather
// than a recursion argument) because the chain can recurse indirectly through
// policy creation -- _add_profile_selectors -> xccdf_policy_model_get_policy_by_id
// -> xccdf_policy_new -> _add_profile_selectors -- which would reset a per-call
// argument and let a cyclic @extends overflow the stack.
static unsigned _xccdf_extends_recursion_depth = 0;

static void _xccdf_policy_add_profile_selectors(struct xccdf_policy* policy, struct xccdf_benchmark *benchmark, struct xccdf_profile *profile) {
if (!profile)
return;

if (_xccdf_extends_recursion_depth > XCCDF_POLICY_MAX_EXTENDS_DEPTH) {

@Mab879 Mab879 Jun 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
if (_xccdf_extends_recursion_depth > XCCDF_POLICY_MAX_EXTENDS_DEPTH) {
if (_xccdf_extends_recursion_depth >= XCCDF_POLICY_MAX_EXTENDS_DEPTH) {

If we are be pedantic about the limit.

oscap_seterr(OSCAP_EFAMILY_XCCDF, "Profile @extends chain is too deep or cyclic; aborting selector resolution for '%s'.",

@Mab879 Mab879 Jun 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It might be better to assign xccdf_profile_get_id(profile) ? xccdf_profile_get_id(profile) : "(unknown)" to a variable and use it so that we don't reference a null here.

xccdf_profile_get_id(profile));
return;
}
_xccdf_extends_recursion_depth++;

const char *parent_profile_id = xccdf_profile_get_extends(profile);
struct xccdf_profile *parent_profile = NULL;

if (parent_profile_id != NULL) {
if (strcmp(parent_profile_id, xccdf_profile_get_id(profile)) == 0) {
if (oscap_strcmp(parent_profile_id, xccdf_profile_get_id(profile)) == 0) {
// We are shadowing a profile, we need to get the original profile from
// benchmark directly to avoid an endless loop.
parent_profile = xccdf_benchmark_get_profile_by_id(benchmark, parent_profile_id);
Expand Down Expand Up @@ -1902,6 +1921,8 @@ static void _xccdf_policy_add_profile_selectors(struct xccdf_policy* policy, str
_xccdf_policy_add_selector_internal(policy, benchmark, clone, false);
}
xccdf_select_iterator_free(sel_it);

_xccdf_extends_recursion_depth--;
}

/**
Expand Down
Loading