Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update codeberg.org/gruf libraries and fix go-store issue #347

Merged
merged 2 commits into from
Dec 20, 2021

Conversation

NyaaaWhatsUpDoc
Copy link
Member

@NyaaaWhatsUpDoc NyaaaWhatsUpDoc commented Dec 15, 2021

Fixes a critical issue in the codeberg.org/gruf/go-store library in which path traversal would have been possible. Fortunately this doesn't work with our HTTP muxer but still something that's worth fixing!

I will leave a comment below in the relevant vendored code that's worth checking.

This has passed my own module's tests but it's still worth checking that this doesn't cause any fileserver path issues on a test instance.

Signed-off-by: kim <grufwub@gmail.com>
Signed-off-by: kim <grufwub@gmail.com>
// than store root, this is an error
if util.CountDotdots(pb.StringPtr()) > st.dots {
// Check for dir traversal outside of root
if util.IsDirTraversal(st.path, pb.StringPtr()) {
Copy link
Member Author

Choose a reason for hiding this comment

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

So here is where the dir traversal check is being performed. Next comment will be the implementation of said function

var dotdot = "../"
// IsDirTraversal will check if rootPlusPath is a dir traversal outside of root,
// assuming that both are cleaned and that rootPlusPath is path.Join(root, somePath)
func IsDirTraversal(root string, rootPlusPath string) bool {
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the dir traversal check function.

@tsmethurst tsmethurst merged commit 635ad2a into superseriousbusiness:main Dec 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants