Skip to content

Conversation

@efrat-starkware
Copy link
Contributor

@efrat-starkware efrat-starkware commented Aug 7, 2025

Type

  • feature
  • bugfix
  • dev (no functional changes, no API changes)
  • fmt (formatting, renaming)
  • build
  • docs
  • testing

Description

Breaking changes?

  • yes
  • no

This change is Reviewable

@efrat-starkware efrat-starkware force-pushed the efrat/compile_cairo_script branch from 5fc32cd to 98d341a Compare August 7, 2025 19:21
.filter_map(Result::ok)
.filter(|e| e.file_type().is_file() && e.path().extension().map_or(false, |ext| ext == "cairo"))
{
let content = fs::read_to_string(entry.path())?;

Choose a reason for hiding this comment

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

Semgrep identified an issue in your code:
The application builds a file path from potentially untrusted data, which can lead to a path traversal vulnerability. An attacker can manipulate the path which the application uses to access files. If the application does not validate user input and sanitize file paths, sensitive files such as configuration or user data can be accessed, potentially creating or overwriting files. To prevent this vulnerability, validate and sanitize any input that is used to create references to file paths. Also, enforce strict file access controls. For example, choose privileges allowing public-facing applications to access only the required files.

Dataflow graph
flowchart LR
    classDef invis fill:white, stroke: none
    classDef default fill:#e7f5ff, color:#1c7fd6, stroke: none

    subgraph File0["<b>crates/cairo-compile-utils/src/lib.rs</b>"]
        direction LR
        %% Source

        subgraph Source
            direction LR

            v0["<a href=https://github.com/starkware-libs/bootloader-hints/blob/98d341a3e5ffab9a225e8524f4f31e02d54e4b3f/crates/cairo-compile-utils/src/lib.rs#L16 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 16] entry.path()</a>"]
        end
        %% Intermediate

        %% Sink

        subgraph Sink
            direction LR

            v1["<a href=https://github.com/starkware-libs/bootloader-hints/blob/98d341a3e5ffab9a225e8524f4f31e02d54e4b3f/crates/cairo-compile-utils/src/lib.rs#L16 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 16] entry.path()</a>"]
        end
    end
    %% Class Assignment
    Source:::invis
    Sink:::invis

    File0:::invis

    %% Connections

    Source --> Sink


Loading

To resolve this comment:

✨ Commit Assistant fix suggestion

Suggested change
let content = fs::read_to_string(entry.path())?;
// Ensure the entry's path is within the intended directory to prevent path traversal
if !entry.path().starts_with(project_root) {
continue; // Skip files outside the target directory
}
let content = fs::read_to_string(entry.path())?;
View step-by-step instructions
  1. Validate that entry.path() only points to files within the intended directory (project_root) by checking that the entry's path starts with project_root. You can do this with entry.path().starts_with(project_root).
  2. Before calling fs::read_to_string(entry.path()), add a check:
    if !entry.path().starts_with(project_root) { continue; }
  3. Alternatively, if you expect only .cairo files in a fixed folder, consider filtering out any files with path components like .. or symlinks that could point outside the target directory, to prevent path traversal.
  4. Reject or skip any files not meeting the above requirements before processing their contents.

This will make sure your code only reads files in your intended directory and avoids unwanted file access.

💬 Ignore this finding

Reply with Semgrep commands to ignore this finding.

  • /fp <comment> for false positive
  • /ar <comment> for acceptable risk
  • /other <comment> for all other reasons

Alternatively, triage in Semgrep AppSec Platform to ignore the finding created by tainted-path.

You can view more details about this finding in the Semgrep AppSec Platform.

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.

2 participants