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

memory corruption in tty_server_new #45

Closed
PerBothner opened this issue Mar 20, 2017 · 2 comments
Closed

memory corruption in tty_server_new #45

PerBothner opened this issue Mar 20, 2017 · 2 comments

Comments

@PerBothner
Copy link

I found a memory corruption error in DomTerm. This was in code imported from ttyd, which appears to have the same bug: The cmd_len value does not include space for the final nul. Alternatively, you could:

ts->command = xmalloc(cmd_len+1);

The second part of the patch is optional - it just seems strange to use sprintf there. Is there some reason for it?

server-patch.txt

@tsl0922
Copy link
Owner

tsl0922 commented Mar 21, 2017

Thanks! This should be a bug, fixed by f533355, and removed the strange sprintf.

@tsl0922 tsl0922 closed this as completed Mar 21, 2017
@PerBothner
Copy link
Author

FWIW Setting the null terminator in line 118 is redundant, since stpcpy sets it, assuming cmd_argc > 0 . (It might still be reasonable to set it, for clarity.)

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

No branches or pull requests

2 participants