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

utils: string: fix the global-buffer-overflow from AddressSanitizer #23

Closed

Conversation

laxiLang
Copy link

@laxiLang laxiLang commented Oct 8, 2024

Uses the FreeBSD implementation to fix the global-buffer-overflow triggered from strlen(src) in strlcpy().

Uses the FreeBSD implementation to fix the global-buffer-overflow
triggered from strlen(src) in strlcpy().

Signed-off-by: Lang Xie <[email protected]>
@laxiLang
Copy link
Author

laxiLang commented Oct 8, 2024

@arnopo can you review this change since you are the author?

@rugeGerritsen
Copy link

@laxiLang , this repo is just a mirror. You should point your fix to https://github.com/OpenAMP/open-amp

@@ -49,5 +50,5 @@ strlcpy(char *dst, const char *src, size_t size)
if (nleft == 0 && size != 0)
*dst = '\0'; /* NUL-terminate dst */

return strlen(src);
return (s - src - 1);
Copy link
Collaborator

Choose a reason for hiding this comment

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

The function has to return the total length of the string it tried to create so the size of the source (https://man.freebsd.org/cgi/man.cgi?strlcpy(3))
The freebsd add in code while (*src++) resulting in same overflow issue than strlen, but perhaps not...
if you detect a potential issue, please, comment OpenAMP/open-amp#620 with your observation that I fix it in the openamp repo

@arnopo
Copy link
Collaborator

arnopo commented Nov 5, 2024

@laxiLang the PR #24 should fix the issue, could you check and close this one if the issue is not reproduced?

@laxiLang
Copy link
Author

laxiLang commented Nov 5, 2024

@laxiLang the PR #24 should fix the issue, could you check and close this one if the issue is not reproduced?

Yes, it works with PR #24 changes.

@laxiLang laxiLang closed this Nov 5, 2024
@laxiLang
Copy link
Author

laxiLang commented Nov 5, 2024

Fixed by PR #24

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 this pull request may close these issues.

3 participants