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

File Session Managment permission problem #3763

Closed
nicowaisman opened this issue Aug 14, 2019 · 3 comments · Fixed by #3975
Closed

File Session Managment permission problem #3763

nicowaisman opened this issue Aug 14, 2019 · 3 comments · Fixed by #3975

Comments

@nicowaisman
Copy link

Please answer these questions before submitting your issue. Thanks!

Dear beego Team,

I would like to report a security vulnerability in Beego's session.

There is a problem with file permission when on the File Session Manager that allows (OS) regular access system to access the session folder and potential read session files.

The problem can be found on sess_file.go on the SessionRead function, where a folder is created based on the provided sid with a 777 permission mask, allowing every use on the system to access the folder, create and remove files.

func (fp *FileProvider) SessionRead(sid string) (Store, error) {
func (fp *FileProvider) SessionRead(sid string) (Store, error) {
filepder.lock.Lock()
defer filepder.lock.Unlock()

err := os.MkdirAll(path.Join(fp.savePath, string(sid[0]), string(sid[1])), 0777)
[...]

It later tries to do a stat to see if the session file is there and open or created the session file based on that. A race condition could be generated where a user will try to create it as soon as the folder was created, owning the file and being able to modify later.

If that is not achieved, there is still a problem with the file creation.
f, err = os.Create(path.Join(fp.savePath, string(sid[0]), string(sid[1]), sid))
according to documentation "Create creates the named file with mode 0666 (before umask), truncating it if it already exists", which could also potentially allow a regular user to read the content of the file (again, from inside the Operating System).

Keep in mind, that similar behavior occurs on the SessionRegenerate function that should be addressed.

However, it's important to notice as the documentation describes that os.MkdirAll will apply umask to the permission mode. In ubuntu, for example, that will end up in 755 permission (unless someone changes it), still that makes the file readable and allow users in the OS to read the file's content.

Please let me know when you have fixed the vulnerability so that I can coordinate my disclosure with yours. For reference, here is a link to Semmle's vulnerability disclosure policy: https://lgtm.com/security#disclosure_policy

Thank you,

@NicoleG25
Copy link

@wy65701436 was this issue ever addressed? Please note that CVE-2019-16354 was assigned to this issue.

mzakariyya added a commit to mzakariyya/beego that referenced this issue May 4, 2020
@flycash flycash linked a pull request Jun 13, 2020 that will close this issue
@flycash
Copy link
Collaborator

flycash commented Jun 13, 2020

I think #3975 resolved this issued. Please help to check it.

@github-actions
Copy link

This issue is inactive for a long time.

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

Successfully merging a pull request may close this issue.

3 participants