Report DoS Vulnerability in Docker Execution#193
Conversation
…erability Creates a SECURITY_ISSUE.md detailing a critical Denial of Service vulnerability in `syscore/src/docker/manager.rs` due to the lack of an execution timeout. Also appends the finding to `.jules/sentinel.md` as required by the security auditor persona boundaries.
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
Bot
added
documentation
📝 WalkthroughWalkthroughThe pull request updates security documentation files. The sentinel markdown adds a newline and documents a Docker container timeout vulnerability. The SECURITY_ISSUE.md file replaces a filesystem vulnerability description with a DoS vulnerability related to unbounded Docker container waiting. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
SECURITY_ISSUE.md (1)
49-49: Update OWASP Top 10 reference to the current 2025 edition.The link references the outdated 2017 edition. Update to the current OWASP Top 10:2025 with the correct URL and mapping (Security Misconfiguration is now A02:2025):
Suggested change
- [OWASP Top 10: Security Misconfiguration](https://owasp.org/www-project-top-ten/2017/A6_2017-Security_Misconfiguration) + [OWASP Top 10: Security Misconfiguration](https://owasp.org/Top10/2025/A02_2025-Security_Misconfiguration/)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@SECURITY_ISSUE.md` at line 49, Replace the outdated OWASP 2017 reference text/link "[OWASP Top 10: Security Misconfiguration](https://owasp.org/www-project-top-ten/2017/A6_2017-Security_Misconfiguration)" with the current OWASP Top 10:2025 wording and URL, updating the label to "A02:2025 - Security Misconfiguration" and the link to the official 2025 page; ensure the inline markdown link text and target URL are both changed so the reference and mapping reflect A02:2025.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@SECURITY_ISSUE.md`:
- Line 7: The wait on the Docker container in ContainerManager::execute (the
call to self.docker.wait_container::<String>(&id, None).next().await) has no
timeout and can hang indefinitely; wrap the wait future with a timeout (e.g.,
tokio::time::timeout) and treat timeout as a failure path that force-stops the
container, then run the existing cleanup logic; ensure you call the Docker APIs
to kill/stop and remove the container (e.g.,
docker.kill_container/remove_container or equivalent methods) on timeout and in
all error/return paths (use a try/finally-style drop or a scope guard) so the
cleanup code around the id is always reachable even if the wait times out.
- Around line 25-43: The timeout branch for the wait_future must synchronously
kill and remove the container instead of relying on later cleanup; inside the
Err(_) branch where you handle the timeout of timeout(Duration::from_secs(10),
wait_future).await, call self.docker.kill_container(&id, None).await (or
equivalent kill API) and then call self.docker.remove_container(&id,
Some(RemoveContainerOptions { force: true, .. })) (or equivalent remove API)
before returning a bounded error from the function—do not fall through to
external cleanup or rely on cleanup_container(); ensure these calls use the same
container id variable and propagate any fatal error as the function result.
---
Nitpick comments:
In `@SECURITY_ISSUE.md`:
- Line 49: Replace the outdated OWASP 2017 reference text/link "[OWASP Top 10:
Security
Misconfiguration](https://owasp.org/www-project-top-ten/2017/A6_2017-Security_Misconfiguration)"
with the current OWASP Top 10:2025 wording and URL, updating the label to
"A02:2025 - Security Misconfiguration" and the link to the official 2025 page;
ensure the inline markdown link text and target URL are both changed so the
reference and mapping reflect A02:2025.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ab80fb80-994b-4014-b738-01eb25c03f9a
📒 Files selected for processing (2)
.jules/sentinel.mdSECURITY_ISSUE.md
| ``` | ||
|
|
||
| Because `filename` is attacker-controlled and unsanitized, an attacker can provide an absolute path (e.g., `/etc/passwd` or `/root/.ssh/authorized_keys`) as the `filename`. `PathBuf::join` will discard the `version_dir` and write the uploaded file contents directly to the attacker-specified absolute path on the host filesystem. | ||
| In `syscore/src/docker/manager.rs`, the `execute` method of `ContainerManager` spawns an ephemeral Docker container to run unauthenticated user-supplied code submitted to the `/api/execute` endpoint. The implementation calls `self.docker.wait_container::<String>(&id, None).next().await;` on line 203 without enforcing any timeout limit. As a result, if the user code contains an infinite loop or indefinitely blocking operation, the container will run forever, and the backend async task will hang permanently. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate route registration and auth middleware around /api/execute
rg -n -C3 --type=rust '/api/execute|execute'
# Locate handler definitions and middleware layers likely tied to execution endpoints
rg -n -C3 --type=rust 'route\(|Router|middleware|auth|authorize|jwt|guard'Repository: Vaiditya2207/OKernel
Length of output: 7390
🏁 Script executed:
cat -n syscore/src/docker/manager.rs | sed -n '170,220p'Repository: Vaiditya2207/OKernel
Length of output: 2635
🏁 Script executed:
cat -n syscore/src/docker/manager.rs | sed -n '220,280p'Repository: Vaiditya2207/OKernel
Length of output: 3179
🏁 Script executed:
cat -n syscore/src/docker/manager.rs | sed -n '105,165p'Repository: Vaiditya2207/OKernel
Length of output: 2694
🏁 Script executed:
rg -n -A 15 "async fn cleanup_container" syscore/src/docker/manager.rsRepository: Vaiditya2207/OKernel
Length of output: 714
Correct line reference to 227 and confirm timeout vulnerability.
In syscore/src/docker/manager.rs, the execute method spawns an ephemeral Docker container to run unauthenticated user-supplied code from the /api/execute endpoint. Line 227 calls self.docker.wait_container::<String>(&id, None).next().await; without enforcing any timeout limit. If user code contains an infinite loop or indefinitely blocking operation, the container will run forever and the backend async task will hang permanently. The cleanup code at line 273 is unreachable if the wait call blocks, leaving zombie containers.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@SECURITY_ISSUE.md` at line 7, The wait on the Docker container in
ContainerManager::execute (the call to self.docker.wait_container::<String>(&id,
None).next().await) has no timeout and can hang indefinitely; wrap the wait
future with a timeout (e.g., tokio::time::timeout) and treat timeout as a
failure path that force-stops the container, then run the existing cleanup
logic; ensure you call the Docker APIs to kill/stop and remove the container
(e.g., docker.kill_container/remove_container or equivalent methods) on timeout
and in all error/return paths (use a try/finally-style drop or a scope guard) so
the cleanup code around the id is always reachable even if the wait times out.
| Implement a strict execution timeout on the container execution to limit its maximum lifespan. Wrap the `wait_container` await with `tokio::time::timeout`. | ||
|
|
||
| Example fix: | ||
| ```rust | ||
| let safe_filename = std::path::Path::new(&filename) | ||
| .file_name() | ||
| .and_then(|name| name.to_str()) | ||
| .ok_or((StatusCode::BAD_REQUEST, "Invalid filename".to_string()))?; | ||
|
|
||
| let file_path = version_dir.join(safe_filename); | ||
| use tokio::time::{timeout, Duration}; | ||
|
|
||
| // ... | ||
| let wait_future = self.docker.wait_container::<String>(&id, None).next(); | ||
|
|
||
| match timeout(Duration::from_secs(10), wait_future).await { | ||
| Ok(Some(Ok(res))) => { | ||
| tracing::debug!("[Job {}] Container exited with code {}", job_id, res.status_code); | ||
| } | ||
| Ok(_) => { | ||
| tracing::warn!("[Job {}] Wait failed or container crashed specifically", job_id); | ||
| } | ||
| Err(_) => { | ||
| tracing::error!("[Job {}] Execution timed out after 10 seconds", job_id); | ||
| // Container cleanup happens below, but we should forcibly kill the container here if needed. | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "SECURITY_ISSUE.md" -o -name "*.md" -type f | head -20Repository: Vaiditya2207/OKernel
Length of output: 509
🏁 Script executed:
cat -n ./SECURITY_ISSUE.md | sed -n '20,50p'Repository: Vaiditya2207/OKernel
Length of output: 1587
🏁 Script executed:
rg -i "wait_container" --type rust -A 10 -B 5Repository: Vaiditya2207/OKernel
Length of output: 1171
🏁 Script executed:
wc -l syscore/src/docker/manager.rsRepository: Vaiditya2207/OKernel
Length of output: 97
🏁 Script executed:
cat -n syscore/src/docker/manager.rs | sed -n '200,305p'Repository: Vaiditya2207/OKernel
Length of output: 4862
🏁 Script executed:
rg -n "cleanup_container" syscore/src/docker/manager.rs -A 15Repository: Vaiditya2207/OKernel
Length of output: 1315
🏁 Script executed:
rg -n "kill_container" syscore/src/docker/manager.rsRepository: Vaiditya2207/OKernel
Length of output: 46
🏁 Script executed:
rg -n "timeout" syscore/src/docker/manager.rsRepository: Vaiditya2207/OKernel
Length of output: 46
On timeout, explicitly kill and remove the container within the error branch—relying on cleanup after is insufficient.
The guidance in SECURITY_ISSUE.md is incomplete. If wait_container times out, the error branch is taken, but the cleanup_container() call at line 8 of the suggested flow is never reached. The container continues running, and the vulnerability persists. Timeout and cleanup must both execute synchronously in the error handler.
The suggested remediation should mandate:
- On timeout, call
docker.kill_container()immediately - Call
docker.remove_container()withforce: true - Return a bounded error—do not rely on downstream cleanup
Corrected guidance
- Implement a strict execution timeout on the container execution to limit its maximum lifespan. Wrap the `wait_container` await with `tokio::time::timeout`.
+ Implement a strict execution timeout with mandatory forced termination on timeout.
+ Wrap `wait_container` with `tokio::time::timeout` and kill/remove the container in the error branch.
match timeout(Duration::from_secs(10), wait_future).await {
Ok(Some(Ok(res))) => {
tracing::debug!("[Job {}] Container exited with code {}", job_id, res.status_code);
}
Ok(_) => {
tracing::warn!("[Job {}] Wait failed or container crashed specifically", job_id);
}
Err(_) => {
tracing::error!("[Job {}] Execution timed out after 10 seconds", job_id);
- // Container cleanup happens below, but we should forcibly kill the container here if needed.
+ // Must forcibly kill and remove the container immediately; downstream cleanup will not execute.
+ let _ = self.docker.kill_container(&id, None).await;
+ let _ = self.docker.remove_container(&id, Some(RemoveContainerOptions { force: true, ..Default::default() })).await;
+ return Err(format!("Container execution timeout"));
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Implement a strict execution timeout on the container execution to limit its maximum lifespan. Wrap the `wait_container` await with `tokio::time::timeout`. | |
| Example fix: | |
| ```rust | |
| let safe_filename = std::path::Path::new(&filename) | |
| .file_name() | |
| .and_then(|name| name.to_str()) | |
| .ok_or((StatusCode::BAD_REQUEST, "Invalid filename".to_string()))?; | |
| let file_path = version_dir.join(safe_filename); | |
| use tokio::time::{timeout, Duration}; | |
| // ... | |
| let wait_future = self.docker.wait_container::<String>(&id, None).next(); | |
| match timeout(Duration::from_secs(10), wait_future).await { | |
| Ok(Some(Ok(res))) => { | |
| tracing::debug!("[Job {}] Container exited with code {}", job_id, res.status_code); | |
| } | |
| Ok(_) => { | |
| tracing::warn!("[Job {}] Wait failed or container crashed specifically", job_id); | |
| } | |
| Err(_) => { | |
| tracing::error!("[Job {}] Execution timed out after 10 seconds", job_id); | |
| // Container cleanup happens below, but we should forcibly kill the container here if needed. | |
| } | |
| Implement a strict execution timeout with mandatory forced termination on timeout. | |
| Wrap `wait_container` with `tokio::time::timeout` and kill/remove the container in the error branch. | |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@SECURITY_ISSUE.md` around lines 25 - 43, The timeout branch for the
wait_future must synchronously kill and remove the container instead of relying
on later cleanup; inside the Err(_) branch where you handle the timeout of
timeout(Duration::from_secs(10), wait_future).await, call
self.docker.kill_container(&id, None).await (or equivalent kill API) and then
call self.docker.remove_container(&id, Some(RemoveContainerOptions { force:
true, .. })) (or equivalent remove API) before returning a bounded error from
the function—do not fall through to external cleanup or rely on
cleanup_container(); ensure these calls use the same container id variable and
propagate any fatal error as the function result.
1 participant
Documents a CRITICAL Denial of Service vulnerability found in the
syscorebackend. The/api/executeendpoint executes user-supplied code inside ephemeral Docker containers but fails to enforce a timeout on thewait_containeroperation. This allows malicious actors to exhaust server resources by submitting infinite loops. A detailed report was generated inSECURITY_ISSUE.mdand added to the Sentinel journal without modifying any application code.PR created automatically by Jules for task 7714072846406067781 started by @Vaiditya2207
Summary by CodeRabbit
Release Notes