Fix critical bugs and stabilize attack surface approximation module - #11
Draft
krpandrei05 wants to merge 23 commits into
Draft
Fix critical bugs and stabilize attack surface approximation module#11krpandrei05 wants to merge 23 commits into
krpandrei05 wants to merge 23 commits into
Conversation
Fixes: open-crs#10 Signed-off-by: Andrei Carp <andrei.krp@gmail.com>
Added a check in the CLI to ensure the --elf option is provided when using binary_pattern_matching, preventing crashes and misleading results. Fixes: open-crs#12 Signed-off-by: Andrei Carp <andrei.krp@gmail.com>
Added commons library as a local path dependency to fix ModuleNotFoundError and updated Docker SDK to 7.1.0 to support modern URL schemes. Fixes: open-crs#13, Fixes: open-crs#15 Signed-off-by: Andrei Carp <andrei.krp@gmail.com>
Replaced the hardcoded 'uname' command with the dynamic CONTAINER_EXECUTABLE from configuration. This ensures the fuzzer analyzes the intended binary. Signed-off-by: Andrei Carp <andrei.krp@gmail.com>
Added a check for argc > 1 before accessing command line arguments in the C tracer. This fixes intermittent crashes when running the binary without arguments during calibration. Signed-off-by: Andrei Carp <andrei.krp@gmail.com>
Introduced a separate counter for executable segments to avoid out-of-bounds writes. Previously, the global map index was used for a restricted array, causing SIGSEGV. Signed-off-by: Andrei Carp <andrei.krp@gmail.com>
Migrated from stack allocation to dynamic allocation (malloc) for the hashed buffer and increased its size. This prevents stack corruption caused by buffer overflow when processing a large number of basic blocks. Signed-off-by: Andrei Carp <andrei.krp@gmail.com>
Added chmod calls to ensure result directories and files created by the root user in Docker are readable by the host Python process. Signed-off-by: Andrei Carp <andrei.krp@gmail.com>
Added a command to remove CMakeCache.txt before compilation. This ensures that changes to the tracer source or headers are correctly reflected in the compiled library. Signed-off-by: Andrei Carp <andrei.krp@gmail.com>
Modified the generator to test simple flags before complex combinations. This prevents valid flags from being ignored due to hash collisions with previously seen invalid combinations. Signed-off-by: Andrei Carp <andrei.krp@gmail.com>
Corrected the return type to bool and added safety checks for null instrumentation hashes in both the validation logic and history tracking. This prevents crashes and incorrect deduplication when Docker runs fail. Signed-off-by: Andrei Carp <andrei.krp@gmail.com>
Decoupled business logic from presentation in cli.py by introducing run_detection and run_fuzzing helpers. Updated the analyze command to collect all results before rendering, ensuring instantaneous output and eliminating visual latency between static and dynamic analysis phases. Signed-off-by: Andrei Carp <andrei.krp@gmail.com>
Pinned all dependencies in pyproject.toml to exact versions to ensure environment reproducibility. Updated the black package to a secure version to resolve two critical security vulnerabilities: arbitrary file write via unsanitized cache filenames and Regular Expression Denial of Service (ReDoS). Fixes: #1, Fixes: #2 Signed-off-by: Andrei Carp <andrei.krp@gmail.com>
krpandrei05
force-pushed
the
krpandrei/fix/bug-fixes-and-stability
branch
from
June 15, 2026 10:04
56536b2 to
b6f0e30
Compare
…known limitations Signed-off-by: Andrei Carp <andrei.krp@gmail.com>
…r output Run each FLAG and STRING_ENABLER candidate without QBDI and check stderr. Arguments that produce stderr output are invalid getopt options and are removed from results. Signed-off-by: Andrei Carp <andrei.krp@gmail.com>
…_arguments Wrapped parser.parse() in try/except ParseError. On failure, falls back to regex search for \bargc\b|\bargv\b on raw decompiled text. Handles Ghidra output containing POSIX types or global pointer artifacts not fully covered by preprocessing in ghidra.py. Signed-off-by: Andrei Carp <andrei.krp@gmail.com>
…g constraints Adds 5 limitations: external library dependencies in QBDI container, dictionary-dependent fuzzing, no combined argument testing, runtime environment requirements, indirect input not detected. Signed-off-by: Andrei Carp <andrei.krp@gmail.com>
…ng hashes Adds __ignores_string_value filter that runs the binary with two different canary strings and compares their hashes. Equal hashes mean the string value is irrelevant to the binary's execution — false STRING_ENABLER. Also renames __is_false_positive to __produces_stderr for clarity. Signed-off-by: Andrei Carp <andrei.krp@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR aims to stabilize the attack_surface_approximation module by improving dictionary generation, static input-stream detection, and the Docker/QBDI-based argument fuzzing pipeline, alongside tightening dependency versions and updating user-facing documentation.
Changes:
- Improves robustness of heuristics and static detection (e.g., man-page parsing error handling; fallback argument detection when decompilation parsing fails).
- Refactors CLI orchestration and adjusts fuzzing logic/filters to reduce false positives and crashes.
- Updates QBDI tracer build/run behavior inside Docker and revises documentation + Poetry dependency pinning.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
README.md |
Updates usage/setup docs and examples for dictionary generation, detection, and fuzzing. |
pyproject.toml |
Pins Python and dependency versions (runtime + dev) for reproducibility/security. |
attack_surface_approximation/static_input_streams_detection/detector.py |
Adds parse-failure fallback for argv/argc detection. |
attack_surface_approximation/dictionaries_generators/heuristics/man_parsing.py |
Prevents crashes when manpages are missing/undecodable. |
attack_surface_approximation/cli.py |
Adds heuristic-specific validation and refactors detect/fuzz/analyze flow. |
attack_surface_approximation/arguments_fuzzing/qbdi_analysis.py |
Adds stderr-check helper, adjusts permissions handling, and changes tracer build/run steps. |
attack_surface_approximation/arguments_fuzzing/qbdi_analysis_scripts/qbdi_preload_template.c |
Addresses potential memory/logic hazards in tracer hashing/segment handling/output writing. |
attack_surface_approximation/arguments_fuzzing/fuzzing_sequence_generator.py |
Reorders fuzzing sequence generation for better flag detection coverage. |
attack_surface_approximation/arguments_fuzzing/fuzzer.py |
Improves hash handling, adds stderr filtering and string-irrelevance filtering, and adjusts dedup logic. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
56
to
+60
| def generate(heuristic: str, output: str, top: int, elf: str = None) -> None: | ||
| if heuristic == "binary_pattern_matching" and elf is None: | ||
| print("[ERROR] The 'binary_pattern_matching' heuristic requires an ELF file. Please provide one using the --elf option.") | ||
| return | ||
|
|
Comment on lines
+74
to
+76
| 4. Ensure the Docker API is accessible by: | ||
| - Running the module as `root`; or | ||
| - Changing the Docker socket permissions (unsecure approach) via `chmod 777 /var/run/docker.sock`. |
|
|
||
| ```console | ||
| $ poetry run attack_surface_approximation generate --heuristic man_parsing --output args.txt --top 100 | ||
| ➜ poetry run attack_surface_approximation generate --heuristic man --output args.txt --top 10 |
Comment on lines
+145
to
+153
| def produces_stderr(self, argument: ArgumentsPair) -> bool: | ||
| stringified_arguments = argument.to_str() | ||
| result = self.__container.exec_run( | ||
| f"timeout {self.timeout} {self.__configuration.CONTAINER_EXECUTABLE} {stringified_arguments}", | ||
| workdir="/home/docker", | ||
| demux=True, | ||
| ) | ||
| _, stderr = result.output | ||
| return bool(stderr) |
Comment on lines
+136
to
+140
| cmake_result = self.__container.exec_run( | ||
| "cmake .", | ||
| workdir=self.__configuration.CONTAINER_SO_FOLDER, | ||
| ) | ||
| self.__container.exec_run( | ||
| make_result = self.__container.exec_run( |
Comment on lines
125
to
+130
| self.__container.exec_run( | ||
| f"mkdir -p {self.__configuration.CONTAINER_RESULTS_FOLDER}" | ||
| ) | ||
| self.__container.exec_run( | ||
| f"sudo chmod 777 {self.__configuration.CONTAINER_RESULTS_FOLDER}" | ||
| ) |
Comment on lines
+213
to
+217
| # Ensure the result file is readable by the host user | ||
| argument_identifier = argument.to_hex_id() | ||
| self.__container.exec_run( | ||
| f"chmod 666 {os.path.join(self.__configuration.CONTAINER_RESULTS_FOLDER, argument_identifier)}" | ||
| ) |
Comment on lines
65
to
+67
| def __check_if_argument_is_valid( | ||
| self, argument: ArgumentsPair, result: QBDIAnalysis | ||
| ) -> None: | ||
| ) -> bool: |
Comment on lines
79
to
83
| arg = FileArgument(self.canary_filename) | ||
| yield arg | ||
| if ArgumentRole.FILE_ENABLER not in arg.get_roles_based_on_analysis( | ||
| self.last_analysis_result, bbs_hashes_baseline | ||
| ): | ||
| for argument in self.arguments: | ||
| yield ArgumentPlusFileArgument(argument, self.canary_filename) | ||
|
|
||
| yield ArgumentArgument("-") | ||
|
|
Adds baseline_uses_stdin and baseline_uses_file captured from the first NoneArgument run in __generate_baseline_hashes. Post-fuzz filters __reads_stdin_regardless and __opens_files_regardless discard STDIN_ENABLER and FILE_ENABLER candidates where the binary exhibits that behavior without any flag. Also makes get_valid_argument private (__get_valid_argument) since it is only called from get_all_valid_arguments. Signed-off-by: Andrei Carp <andrei.krp@gmail.com>
…etection Add tty=True to exec_run in __build_and_run_analyze_command so the analyzed binary receives a real PTY as stdin. Without this, exec_run provides EOF stdin causing any binary reading stdin to return immediately without blocking — making timeout-based STDIN_ENABLER detection always return False. Also fix type bug in __detect_stdin_usage: recursive analyze() call returned QBDIAnalysisResult instead of bool; corrected to .uses_stdin. Signed-off-by: Andrei Carp <andrei.krp@gmail.com>
Add __enter__/__exit__ to QBDIAnalysis and ArgumentsFuzzer so the Docker container is always removed on exit, even when exceptions occur. Update run_fuzzing in cli.py to use 'with ArgumentsFuzzer(...) as fuzzer'. Remove the previously commented-out __del__ method. Signed-off-by: Andrei Carp <andrei.krp@gmail.com>
…used The for loop calling __check_parameters_used(attr) passed attribute names (strings) instead of their values, making the recursion a no-op. AST traversal is already handled by super().generic_visit(node) in generic_visit, so the loop was redundant. Removed entirely. Signed-off-by: Andrei Carp <andrei.krp@gmail.com>
… limitation Signed-off-by: Andrei Carp <andrei.krp@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This Pull Request introduces a series of critical fixes required to make the
attack_surface_approximationmodule fully functional and stable.Key changes include:
man_parsing,binary_pattern_matching).This is the final update aimed at stabilizing the execution environment and module dependencies using Poetry, ensuring full compatibility with the OpenCRS framework.
Fixes: #10, Fixes: #12, Fixes: #13, Fixes: #15
This PR addresses the following Dependabot alerts:
blackblack