Security: sanitize/replace shell invocations to avoid injection
Nobody has claimed this yet.
- Dominant language
- PHP
- Stars
- 755
- Forks
- 128
- PR merge metrics
- No merged PRs in 30d
Description
Potential command injection risk in shell invocations that include user-controlled values without shell-escaping.
Examples
-
openml_OS/helpers/api_helper.phpvalidate_arff(): builds asedcommand using$nameand$didto 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);$nameoriginates from dataset metadata and may contain characters that break quoting. This should avoid shell entirely. -
openml_OS/controllers/Api_splits.php: multiplesystem()/exec()calls build Java commands with request-derived inputs. Some checks (is_safe,is_numeric) exist, but defense-in-depth suggests escaping args or usingproc_openwith argv arrays.
Proposed fix
- Replace the
sedcall 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.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with openml_OS/helpers/api_helper.php, especially validate_arff(), and inspect the command construction shown in the issue. Then review shell calls in openml_OS/controllers/Api_splits.php and the existing is_safe/is_numeric checks. Done means the prepend operation avoids shell invocation, all user-provided command values are protected, and unit tests cover edge-case names.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java, php
- Domain
- backend, security
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100