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

Fixed c4996 compiler warning/error due to use of strncpy and ace_clipboard memory leak #3331

Closed
wants to merge 9 commits into from

Conversation

looterz
Copy link
Contributor

@looterz looterz commented Feb 17, 2016

Noticed the funny comment and thought I would submit a PR, strncpy_s with the _TRUNCATE flag will also ensure that the string is null-terminated whether it fits or not. Looks like ballistics and some other extensions make heavy use of strncpy as well but don't disable the warning, which on some compilers might cause an error.

@PabstMirror PabstMirror added the kind/cleanup Release Notes: **CHANGED:** label Feb 17, 2016
@PabstMirror
Copy link
Contributor

Looks good, extensions parse_imagepath and fcs both have the same problem

@looterz
Copy link
Contributor Author

looterz commented Feb 17, 2016

Okay, patched every module using strcpy, strncpy or in clipboards case memcpy for strings. Didn't come across any code that looked like you could actually trigger an overflow which is good.

@looterz
Copy link
Contributor Author

looterz commented Feb 18, 2016

Fixed some issues I spotted with the ace_clipboard implementation, such as using memory semantics meant for GMEM_MOVEABLE, and the heap memory not being cleaned up on failure and being allocated before the clipboard was ready to be used.


GlobalUnlock(hClipboardData);
#ifndef _WIN32
EXTENSION_RETURN();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't the extension always return here before running the strcmp check?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only if the pre-processor definition _WIN32 is undefined, so basically only if compiled/used on linux. I was looking into getting clipboard content from x11 for ubuntu support, but im not sure where ACE is with linux support.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, sorry, I missed the n

@looterz looterz changed the title Fixed c4996 compiler warning/error due to use of strncpy Fixed c4996 compiler warning/error due to use of strncpy and ace_clipboard memory leak Feb 27, 2016
@thojkooi thojkooi added this to the 3.6.0 milestone May 7, 2016
@thojkooi thojkooi self-assigned this May 7, 2016
@thojkooi
Copy link
Contributor

thojkooi commented May 8, 2016

This looks good, just needs some formatting changes and the merge conflict resolved. Once that is done, we will merge it 👍 Thanks @looterz

I wil see if I can get around to do that later today.

@thojkooi thojkooi mentioned this pull request Jun 12, 2016
@thojkooi
Copy link
Contributor

#3905

@thojkooi thojkooi closed this Jun 12, 2016
@thojkooi thojkooi removed this from the 3.6.0 milestone Jun 12, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/cleanup Release Notes: **CHANGED:**
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants