[Low] CSI plugin builds staging directories from storage_volume_id without a containment check
Maintainers usually reply within 1 day
Nobody has claimed this yet.
Assessment
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Newbie friendliness
- 76/100
Research direction
Read internal/volume/csi/plugin.go at the stageVolume and unstageVolume path construction sites, then check how resources.ValidateStorageVolumeID validates IDs. Add coverage for ../x and a/b in the relevant unit test and verify neither staging operation accesses paths outside the staging prefix.
Written by the indexing model from the issue text.
Description
Summary
stageVolume and unstageVolume in the CSI volume plugin build the staging path as filepath.Join(p.stagingDirPrefix, volumeID), then MkdirAll and later remove it. storage_volume_id is only checked for control characters and length, so a value containing / or .. would resolve outside stagingDirPrefix. The ID comes from the CSI provisioner and control plane, not from tenants, so this is defense in depth.
Details
internal/volume/csi/plugin.go#L296-L299:
stagingPath := filepath.Join(p.stagingDirPrefix, req.VolumeID)
if err := os.MkdirAll(stagingPath, 0750); err != nil {
return "", fmt.Errorf("failed to create staging directory %q: %w", stagingPath, err)
}
#L344: stagingPath := filepath.Join(p.stagingDirPrefix, volumeID) (unstage, followed by removal of the staging dir).
storage_volume_id (pkg/proto/ateapipb/ateapi.proto#L507-L519) is "the storage system's own" format, validated only by resources.ValidateStorageVolumeID (control characters).
Impact
Low: the value is set by the provisioner when it creates the volume, not by an actor or template. A misbehaving or compromised CSI driver could return an ID that makes atelet create or remove directories outside the staging prefix on the node.
Suggested fix
Derive the staging directory from a hash of the volume ID (as kubelet does) instead of the raw ID, or reject IDs where !filepath.IsLocal(id). Add a unit test with ../x and a/b.
- Dominant language
- Go
- Stars
- 5k
- Forks
- 563
- Avg merge
- 2d 12h
- Merged PRs (30d)
- 493
Getting set up
- No Dockerfile or Docker Compose file
- Has a pull request template
- Read the contributing 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.
More from agent-substrate/substrate
-
area/security kind/bug
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
agent-substrate/substrate#2932 ·
Maintainers usually reply within 1 day
-
area/security kind/bug
Difficulty 2/5 1-3 hours Newbie friendliness 76/100
agent-substrate/substrate#2920 ·
Maintainers usually reply within 1 day
-
area/security kind/bug
Difficulty 2/5 1-3 hours Newbie friendliness 73/100
agent-substrate/substrate#2919 ·
Maintainers usually reply within 1 day
-
area/security kind/bug
Difficulty 2/5 1-3 hours Newbie friendliness 75/100
agent-substrate/substrate#2918 ·
Maintainers usually reply within 1 day
-
area/dev-infra kind/docs kind/feature
Difficulty 2/5 1-3 hours Newbie friendliness 72/100
agent-substrate/substrate#2875 ·
Maintainers usually reply within 1 day
All issues in agent-substrate/substrate
Similar issues
-
Difficulty 2/5 1-3 hours Newbie friendliness 76/100
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 82/100
-
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
-
bug
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
Maintainers usually reply within 2 days
-
bug
Difficulty 2/5 1-3 hours Newbie friendliness 83/100
devlikeapro/waha#2322 ·
Maintainers usually reply within 2 days