Skip to content

Appeng 5612 and 5613#284

Open
RedTanny wants to merge 6 commits into
RHEcosystemAppEng:mainfrom
RedTanny:APPENG-5612-5613
Open

Appeng 5612 and 5613#284
RedTanny wants to merge 6 commits into
RHEcosystemAppEng:mainfrom
RedTanny:APPENG-5612-5613

Conversation

@RedTanny

@RedTanny RedTanny commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

Summary

Split from PR #261

  • APPENG-5612: Add argument-count pre-filter to C parser to reject false-positive caller-callee matches
  • APPENG-5613: Add Go sub-package awareness to Function Locator and CCA for precise package matching

APPENG-5612: C CCA Argument-count Pre-filter

Add argument-count pre-filter to C parser's search_for_called_function to reject cross-package false positives where a same-named function has different arity.

Example: rsync read_byte(1 param) vs PostgreSQL read_byte(3 params) — without this filter, both would match incorrectly.

Changes

  • Add _count_c_declared_params() to count declared parameters in C function definitions
  • Add _count_all_call_site_args() to count arguments at ALL call sites (not just first)
  • Handle string/char literals correctly (commas inside "a,b" not counted as separators)
  • Modify search_for_called_function to reject mismatched arity

APPENG-5613: Go CCA Sub-package Awareness

Add Go sub-package awareness so github.com/lib/foo/bar correctly matches target github.com/lib/foo.

Changes

  • Add _extract_go_subpackage() and _go_subpackage_flow_control() to Function Locator
  • Add resolve_subpackage_to_module() in Go parser for CCA sub-package filtering
  • Add extract_go_subpackages_from_patch() in intel_utils for patch enrichment
  • Fix Go FL short_name dict collision: store list of packages per short name
  • Fix is_package_imported regex: remove trailing .* that matched identifier anywhere in path

Review Comments Addressed

All review comments from PR #261 relevant to these tasks have been addressed:

Issue Description Status
#1 module_path receives unresolved short name ✅ Fixed
#2 Comma counting ignores string/char literals ✅ Fixed
#3 Guidance string appended to function-name list ✅ Fixed
#8 is_package_imported regex trailing .* ✅ Fixed
#10 First call site only ✅ Fixed
#11 Missing ValueError guard ✅ Fixed

Test Plan

  • C parser tests pass: pytest src/exploit_iq_commons/utils/functions_parsers/tests/test_c_parser.py -v
  • Go parser tests pass: pytest src/exploit_iq_commons/utils/functions_parsers/tests/test_go_parser.py -v
  • Go subpackage tests pass: pytest tests/test_go_subpackage_fixes.py -v
  • Full test suite passes: pytest --tb=short

Files Changed

File Changes
c_lang_function_parsers.py +158 lines - arg-count pre-filter
golang_functions_parsers.py +28 lines - sub-package support, regex fix
function_name_locator.py +115 lines - Go sub-package flow control
intel_utils.py +145 lines - Go sub-package extraction from patches
test_c_parser.py +1053 lines - new test file
test_go_parser.py +1242 lines - new test file
test_go_subpackage_fixes.py +443 lines - new test file

Total: 7 files, ~3000 insertions

Jira

RedTanny and others added 2 commits July 8, 2026 14:40
APPENG-5612: Add argument-count pre-filter to C parser
- _count_call_site_args now correctly skips commas inside string/char literals
- search_for_called_function checks ALL call sites, accepts if any matches
- Adds helper functions _extract_call_site_args_str, _count_args_in_str, _count_all_call_site_args

APPENG-5613: Add Go sub-package awareness and filtering
- resolve_subpackage_to_module in Go parser for sub-package resolution
- _go_subpackage_flow_control with try/except ValueError guard
- _extract_go_subpackage for extracting full import paths
- extract_go_subpackages_from_patch in intel_utils
- Fixed module_path to use resolved full path instead of short name

Bug fixes from PR RHEcosystemAppEng#261 review:
- C: Fixed string literal handling in argument counting
- C: Now checks all call sites instead of only the first
- Go: Added missing ValueError catch in _go_subpackage_flow_control
- Go: module_path now receives resolved full module path

Split from: RHEcosystemAppEng#261

Signed-off-by: Stanny <stanny@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
- Fix type mixing in _go_subpackage_flow_control: guidance now single-line,
  placed first in result list, detailed info logged for debugging
- Fix is_package_imported regex: remove trailing .* that matched identifier
  anywhere in import path instead of at the end

Addresses review comments from PR RHEcosystemAppEng#261 split (APPENG-5612-5613)

Signed-off-by: Stanny <stanny@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@vbelouso

vbelouso commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Code Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

RedTanny and others added 2 commits July 8, 2026 17:45
…d modules

The trailing .* in the regex is intentional to match identifiers in:
- Versioned module paths (e.g., 'jwt' in 'github.com/golang-jwt/jwt/v5')
- Hyphenated package names (e.g., 'jose' in 'gopkg.in/go-jose/go-jose.v2')

Removing it breaks legitimate use cases. False positives only add extra
candidates to verify, not incorrect final results.

Fixes CI test failures in test_go_parser.py

Signed-off-by: Stanny <stanny@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
…ting

Cherry-pick from PR RHEcosystemAppEng#261 inadvertently reverted the parse_cpe fix from
PR RHEcosystemAppEng#276. This restores the proper implementation that:
- Validates CPE 2.3 format (rejects invalid prefixes/field counts)
- Handles escaped colons in product names (e.g., bar\:mumble)
- Returns (None, None, None, None) for malformed input

Co-authored-by: Cursor <cursoragent@cursor.com>
@RedTanny

RedTanny commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator Author

/test-heavy

@zvigrinberg

zvigrinberg commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

Hi @RedTanny

There is a go dataset item regression in the confusion matrix, was identified correctly before as not exploitable, and now it's wrongly identifies it as exploitable, After discussing it with claude code and investigation, this is what we have found:

Regression: CVE-2022-24769 false positive in openshift/oc (Go Confusion Matrix)

After this PR, the agent incorrectly classifies CVE-2022-24769 as exploitable in openshift/oc (commit f93da179), when the correct verdict is not exploitable.

Root Cause

The is_package_imported fix in golang_functions_parsers.py (lines 641-661) correctly fixes the broken split(r"\s")split(). However, this unmasks a broader issue: CCA now correctly detects that containerd/containerd/sys and containerd/containerd/errdefs are imported in openshift/oc (via docker/docker/pkg/archive and docker/docker/errdefs). CCA traces call chains through these utility sub-packages, finds reachable functions, and returns True.

The problem is that CVE-2022-24769 is about containerd's container creation and capability-setting code (in containerd/containerd/oci), which is not vendored in openshift/oc at all. CCA operates at the module level (containerd/containerd) and matches unrelated utility sub-packages.

Suggested Fix: Narrow Go candidates to patch-derived sub-packages

PR #284 already added extract_go_subpackages_from_patch() in intel_utils.py which identifies the exact sub-packages modified by the CVE fix. This data is appended to critical_context as guidance text, but it's not used to scope the actual CCA query. The fix connects these two pieces:

File 1: src/vuln_analysis/utils/intel_utils.py

Add candidate_packages parameter to enrich_vulnerable_functions_from_patch and change return type to set[str] | None:

async def enrich_vulnerable_functions_from_patch(
    cve_intel_list,
    critical_context: list[str],
    vulnerable_functions: set[str],
    ecosystem: str = "",
    patch_result=None,
    candidate_packages: list[dict] | None = None,   # ADD
) -> set[str] | None:                                # CHANGE from -> None

At each return point inside the function where all_funcs is found, extract and return Go sub-packages:

    # Pre-fetched patch path:
    if patch_result and patch_result.parsed_patch:
        all_funcs = extract_functions_from_parsed_patch(patch_result.parsed_patch, ecosystem)
        if all_funcs:
            go_subpkgs = _extract_go_subpackages_from_result(
                patch_result.parsed_patch, candidate_packages)
            _append_enrichment_to_context(all_funcs, critical_context,
                                          f"pre-fetched patch ({patch_result.source})",
                                          go_subpackages=go_subpkgs)
            return go_subpkgs    # was: bare return

    # Same pattern for the fetched-patch path (~line 613).

File 2: src/vuln_analysis/functions/cve_agent.py

In _process_steps(), use the returned Go sub-packages to narrow candidate_packages before building precomputed_intel:

    go_subpackages = None
    if not vulnerable_functions:
        vf_set: set[str] = set()
        pre_fetched_patch = workflow_state.patch_results.get(vuln_id)
        go_subpackages = await enrich_vulnerable_functions_from_patch(
            workflow_state.cve_intel, critical_context, vf_set, ecosystem,
            patch_result=pre_fetched_patch,
            candidate_packages=candidate_packages,
        )
        if vf_set:
            vulnerable_functions = sorted(vf_set)

    # When patch data identifies specific Go sub-packages, narrow candidates
    # so CCA targets the actual vulnerable sub-package (e.g. containerd/oci)
    # instead of the entire module (e.g. containerd/containerd), which would
    # false-match unrelated utility sub-packages like containerd/sys.
    if go_subpackages and ecosystem == "go":
        go_modules = {
            cp["name"] for cp in candidate_packages
            if cp.get("ecosystem", "").lower() == "go"
        }
        sub_candidates = []
        for subpkg in go_subpackages:
            for mod in go_modules:
                if subpkg.startswith(mod + "/") or subpkg == mod:
                    sub_candidates.append({
                        "name": subpkg, "ecosystem": "go",
                        "source": "patch_subpackage",
                    })
                    break
        if sub_candidates:
            non_go = [cp for cp in candidate_packages
                      if cp.get("ecosystem", "").lower() != "go"]
            candidate_packages = non_go + sub_candidates

    precomputed_intel = (critical_context, candidate_packages, vulnerable_functions)

How this fixes CVE-2022-24769

  1. enrich_vulnerable_functions_from_patch extracts {"github.com/containerd/containerd/oci"} from the CVE's patch file paths
  2. cve_agent.py replaces the broad candidate github.com/containerd/containerd with the specific github.com/containerd/containerd/oci
  3. Function Locator checks if containerd/containerd/oci is present in openshift/oc — it's not (only containerd/sys and containerd/errdefs are vendored)
  4. FL reports "package not valid" → package_validated = False → early exit → "not exploitable"

The is_package_imported fix remains correct. CCA just needs to be scoped to the right sub-package.

@zvigrinberg

Hi — thanks for the detailed write-up. I re-ran CVE-2022-24769 on openshift/oc @ f93da179 on this branch and checked the traces + the cached repo. A few corrections vs the proposed RCA:

What this re-run actually produced

  • Verdict was uncertain / “likely not exploitable”, not a hard exploitable false positive.
  • Candidate package was github.com/docker/docker (Moby / GHSA-2mm7-x5h6-5pvq), not containerd.
  • Call Chain Analyzer was never invoked (no_cca_warning: true). Empty Function Caller Finder → finish blocked → forced finish, without CCA.
  • So the “CCA traced through containerd/sys + errdefs → True” path is not what happened in this run.

Ground truth in the oc tree

  • Vulnerable Moby code (docker/docker/oci, daemon, DefaultLinuxSpec / execSetPlatformOpt) is not in vendor / import closure.
  • Only utility slices are present (pkg/archive, errdefs, api/types, …).
  • containerd is vendored only as utils (sys, errdefs, …); containerd/oci is absent. App code does not import containerd.
  • oci existing in the full Go module cache does not mean it is part of this app — Go only keeps packages on the import graph. go list -deps confirms oci/daemon are not in the closure.

Expected outcome for this case is a clear not exploitable. Uncertain is still a bad result, but it is a different failure mode than “wrongly exploitable via containerd CCA.”

Where the agent actually went wrong

  1. Checklist asked runtime/config questions (version, inheritable caps, file caps, users/groups) instead of the mandated function-first reachability question — despite patch hints for DefaultLinuxSpec / execSetPlatformOpt.
  2. Function Locator returned Package is valid + weak fuzzy names (Deadline, IsDefault, …) and INFO that read like DefaultLinuxSpec was found in utility sub-packages. That is misleading: the queried function was not found (oci isn’t indexed).
  3. Observation then invented
    VALIDATED: github.com/docker/docker,DefaultLinuxSpec exists
    and “found in multiple sub-packages…” — upgrading fuzzy/module-only output into a false exact hit. That polluted memory and derailed reachability.
  4. Reachability never called CCA after empty FCF (against Rule 8), then summarize hedged into uncertain.

Note: CVE-2022-24769 is also used by a related containerd advisory (GHSA-c9cp-9c75-9v8c). The containerd/sys / errdefs / oci story fits that sibling more than this Moby/docker/docker path.

On the suggested fix (narrow candidates to patch sub-packages)

Patch-derived sub-package narrowing (e.g. force …/oci) is still reasonable hardening and could help FL fail closed when oci isn’t present. I’m not rejecting the idea.

But I don’t think we should treat the containerd/CCA-exploitable narrative as the RCA for this case without the confusion-matrix trace that produced exploitable. If you still have that run/artifact, please share it — happy to compare.

What we’re planning next (separate from accepting that RCA as-is)

Tighten FL + observation so this can’t be misread again:

  • FL emits clear hooks: package present vs function exact / absent / weak fuzzy (no “found in sub-packages” unless exact/strong match).
  • Observation parses those hooks only for Function Locator (deterministic path), and stops inventing VALIDATED: on fuzzy results.

If the matrix still shows exploitable for this cell, let’s align on the exact job/trace first; my local evidence doesn’t match that claim.


@zvigrinberg zvigrinberg self-requested a review July 9, 2026 04:48
@RedTanny

RedTanny commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator Author

/test-heavy

@zvigrinberg zvigrinberg left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hi @RedTanny
Please see my comments.
Thank you!.

Comment on lines +994 to +1011
# ---------------------------------------------------------------------------
# __trace_down_package (tested indirectly via search_for_called_function)
# ---------------------------------------------------------------------------

class TestTraceDownPackage:
"""Tests for __trace_down_package, exercised indirectly through
search_for_called_function.

When the caller has a local variable whose type belongs to the callee
package, __trace_down_package should resolve the type and return True."""

def setup_method(self):
self.parser = GoLanguageFunctionsParser()

def _doc(self, content, source="test.go"):
return Document(page_content=content, metadata={"source": source})

# Tested in TestGoIsTreeKeyMatch.test_both_empty_strings below

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@RedTanny There is already TestTraceDownPackage class in this test module

Comment on lines +670 to +671
callee_function: Document,
callee_function_package: str,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@RedTanny Until now it was different then the signature of the Parent ABC huh?
But probably it wasn't mattered too much, as C using BFS instead of DFS in generic algorithm of CCA

return counts[0] if counts else None

# Compile the function pattern with recursive param matching
_FUNCTION_PATTERN_REGEX = regex.compile(r'''

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@RedTanny Better relocate this regex pattern to the top of the module, so it will be before the function that uses that, for a better readability of the logic and code.

# Ubuntu's CVE API is often flaky (503/504); fail faster than other intel sources
self._ubuntu_client = UbuntuClient(session=self._session,
retry_count=max_retries,
retry_count=5,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@RedTanny Why not changing to 1 ? :)
Just kidding, maybe we should revert this to the parameter to dictates the max_retries parameter.

from vuln_analysis.utils.prompt_factory import FL_EXAMPLES

logger = LoggingFactory.get_agent_logger(f"exploit-iq.{__name__}")
logger = LoggingFactory.get_agent_logger(f"morpheus.{__name__}")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@RedTanny Please revert

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants