Skip to content

Security: sanitize/replace shell invocations to avoid injection #1265

@lucifer4330k

Description

@lucifer4330k

Potential command injection risk in shell invocations that include user-controlled values without shell-escaping.

Examples

  • openml_OS/helpers/api_helper.php validate_arff(): builds a sed command using $name and $did to prepend info to an ARFF file:

    $info = '% Data set "'.$name.'". ... d/'.$did;
    $string = '1s/^/'.$info.'\n/';
    $command2 = "sed -i -e '$string' $newUrl";
    exec(CMD_PREFIX . $command2, $res, $code);

    $name originates from dataset metadata and may contain characters that break quoting. This should avoid shell entirely.

  • openml_OS/controllers/Api_splits.php: multiple system()/exec() calls build Java commands with request-derived inputs. Some checks (is_safe, is_numeric) exist, but defense-in-depth suggests escaping args or using proc_open with argv arrays.

Proposed fix

  • Replace the sed call with pure PHP file I/O to prepend the info line.
  • When shelling out to Java, strictly validate and/or escape all args (e.g., via escapeshellarg) and prefer argv arrays.
  • Add unit tests covering edge-case names.

Acceptance criteria

  • No use of unescaped user-provided values in shell commands.
  • Prepend operation implemented without shell invocation.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions