-
Notifications
You must be signed in to change notification settings - Fork 23
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
path traversal if path contains rootbucket #221
Comments
Looking at This is, the resource type |
so then the actual error is that the WriteToInboxImpl.java doesnt prefix the rootbucket ? |
Not seeing the error in https://github.com/adorsys/datasafe/blob/develop/datasafe-inbox/datasafe-inbox-impl/src/main/java/de/adorsys/datasafe/inbox/impl/actions/WriteToInboxImpl.java. Can you elaborate? |
wel it doesnt make sure the request is prefixed wtih the root bucket . we'Re using |
Hello @francis-pouatcha this change implies indirectly ensuring that the file paths in the generateUserWithInboxAndOutbox function and related test methods are correctly set up. As it is crucial to avoid NoSuchFileException , are there any other ways we can handle this ? |
https://github.com/adorsys/datasafe/blob/develop/datasafe-storage/datasafe-storage-impl-s3/src/main/java/de/adorsys/datasafe/storage/impl/s3/StaticBucketRouter.java#L27
if the path provided as DocumentFQN to methods like storeDocument contains the rootbucket string in any way (even if it isnt at the start of the string )
everything in front of it will be stripped and allow writing to arbitrary files if any part of the path is user controlled by prefixing the user controlled part with a rootbucket and then adding the remaining folder structure completely ignoring any kind of user restrictions and allowing overwriting of system or other user's files
example
rootbucket : "bucket"
datasafepath: "users/myuserid/private/files/usercontrolled.aes"
vulnerable datasafepath: "users/myuserid/private/files/bucket/users/otheruser/private/files/somefile.aes"
#@ing some people as it seems kinda important :)
@max402 @jkroepke @valb3r
The text was updated successfully, but these errors were encountered: