Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

vc: do not follow symlink when umounting contanier host path #2475

Merged
merged 2 commits into from
Feb 21, 2020

Conversation

bergwolf
Copy link
Member

So that if a guest changes it, we do not end up
propagating the error.

Fixes: #2474

So that if a guest changes it, we do not end up
propergating the error.

Fixes: kata-containers#2474
Signed-off-by: Peng Tao <bergwolf@hyper.sh>
@devimc
Copy link

devimc commented Feb 19, 2020

/test

@codecov
Copy link

codecov bot commented Feb 19, 2020

Codecov Report

❗ No coverage uploaded for pull request base (master@44b0967). Click here to learn what that means.
The diff coverage is 100%.

@@            Coverage Diff            @@
##             master    #2475   +/-   ##
=========================================
  Coverage          ?   50.79%           
=========================================
  Files             ?      114           
  Lines             ?    16381           
  Branches          ?        0           
=========================================
  Hits              ?     8321           
  Misses            ?     7035           
  Partials          ?     1025

@codecov
Copy link

codecov bot commented Feb 19, 2020

Codecov Report

❗ No coverage uploaded for pull request base (master@44b0967). Click here to learn what that means.
The diff coverage is 41.66%.

@@            Coverage Diff            @@
##             master    #2475   +/-   ##
=========================================
  Coverage          ?   50.78%           
=========================================
  Files             ?      114           
  Lines             ?    16392           
  Branches          ?        0           
=========================================
  Hits              ?     8325           
  Misses            ?     7041           
  Partials          ?     1026

A malicious can trick us with a crafted container
rootfs symlink and make runtime umount other mountpoints.
Make sure we do not walk through symlinks when umounting.

Signed-off-by: Peng Tao <bergwolf@hyper.sh>
@bergwolf
Copy link
Member Author

/test

if dest, err := filepath.EvalSymlinks(destination); err == nil && !strings.HasPrefix(dest, filepath.Clean(topDir)) {
return fmt.Errorf("Invalid mount destination %s must not escape", destination)
}

Copy link
Contributor

Choose a reason for hiding this comment

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

there is a "TOCTOU" here

Copy link
Member Author

Choose a reason for hiding this comment

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

I dropped it as it also breaks vm template.

@bergwolf
Copy link
Member Author

/test

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

harden container hostpath cleanup
5 participants