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

Doctor: Permission checks can cause buffer overflow #801

Closed
vikaspotluri123 opened this issue Aug 24, 2018 · 3 comments · Fixed by #804
Closed

Doctor: Permission checks can cause buffer overflow #801

vikaspotluri123 opened this issue Aug 24, 2018 · 3 comments · Fixed by #804
Assignees

Comments

@vikaspotluri123
Copy link
Member

vikaspotluri123 commented Aug 24, 2018

If the Permission checks in Ghost Doctor fail, every file / directory that's not valid will be printed to stdout. If there are a large number of files (ceiling unknown), the error message can get so large it exceeds the allocated buffer for output.

This was originally reported in the forum: https://forum.ghost.org/t/ghost-update-stdout-maxbuffer-exceeded/2635

Edit: The max buffer issue pertains to the stdout of the child process, not of the CLI process - so instead we should be doing something like (although not exactly)

execa(...).catch(error => {
	if (error.message.test(/allocated buffer/) {
		return Promise.resolve({stdout: '[Too many items to list]'});
}).then(result => ...).catch(...);
@acburdine
Copy link
Member

another idea I had to fix this was to setup the child process to pipe output rather than buffer it.

const proc = execa.shell(cmd, { stdout: 'pipe' });

proc.stdout.on('data' () => {}); // do stuff with data

@vikaspotluri123
Copy link
Member Author

I was thinking about that as well, but my thought then the user will have a super long stream of invalid files being listed, which might end up causing issues with their terminal

@acburdine
Copy link
Member

@vikaspotluri123 sorry I should have explained better - the idea I had was to reduce the output in general by showing the specific folders under content that have permissions issues. For example:

Ghost-CLI has detected permissions issues with files in the following folders:
- ./content/uploads/
- ./versions/
...etc

We suggest that the user run a find command on the folders anyways, so it doesn't make sense in my mind to output every single file that has a permissions issue. Using a pipe, it should be pretty easy to determine the top-level folders that have issues.

@acburdine acburdine self-assigned this Aug 28, 2018
acburdine added a commit to acburdine/Ghost-CLI that referenced this issue Aug 29, 2018
closes TryGhost#801
- set maxBuffer size to infinity
acburdine added a commit that referenced this issue Aug 29, 2018
closes #801
- set maxBuffer size to infinity
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 a pull request may close this issue.

2 participants