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

Handling changed file permissions #2285

Closed
liamsi opened this issue Aug 27, 2018 · 2 comments
Closed

Handling changed file permissions #2285

liamsi opened this issue Aug 27, 2018 · 2 comments
Labels
C:libs Component: Library T:security Type: Security (specify priority)

Comments

@liamsi
Copy link
Contributor

liamsi commented Aug 27, 2018

In autofile and in fsdb files are created with read-write permissions for the file owner (0600). If they are opened again, we do not check if the same restrictve file permissions still hold:

func (af *AutoFile) openFile() error {
file, err := os.OpenFile(af.Path, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0600)
if err != nil {
return err
}
af.file = file
return nil
}

and:

tendermint/libs/db/fsdb.go

Lines 202 to 206 in 013b9ce

func write(path string, d []byte) error {
f, err := os.OpenFile(path, os.O_CREATE|os.O_WRONLY, keyPerm)
if err != nil {
return err
}

We should at least check that the permissions did not change (e.g. by accident) and enforce the intended perms (via chmod). Another approach would be to error in this case and tell the user to change the file perms (to make the user aware that sth. funky has happend here).

Will submit a small PR for both cases which silently fixes the permission in case they were changed.

(see https://github.com/tendermint/security/issues/5 and https://github.com/tendermint/security/issues/1)

@liamsi liamsi added T:security Type: Security (specify priority) C:libs Component: Library labels Aug 27, 2018
@melekes
Copy link
Contributor

melekes commented Sep 4, 2018

See #2286 (comment)

@melekes melekes self-assigned this Nov 5, 2018
melekes added a commit that referenced this issue Nov 5, 2018
current code results in panic and we certainly don't want that.
#2286 (comment)
melekes added a commit that referenced this issue Nov 5, 2018
current code results in panic and we certainly don't want that.
#2286 (comment)
enlight pushed a commit to enlight/tendermint that referenced this issue Nov 6, 2018
current code results in panic and we certainly don't want that.
tendermint#2286 (comment)
ebuchman pushed a commit that referenced this issue Nov 16, 2018
current code results in panic and we certainly don't want that.
#2286 (comment)
@melekes melekes removed their assignment Dec 19, 2018
@melekes melekes closed this as completed Apr 24, 2020
@melekes
Copy link
Contributor

melekes commented Apr 24, 2020

Fixed in #2286

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:libs Component: Library T:security Type: Security (specify priority)
Projects
None yet
Development

No branches or pull requests

2 participants