Skip to content

Conversation

@giallu
Copy link
Contributor

@giallu giallu commented Oct 17, 2025

the mystrncpy() function as is written now:

int mystrncpy(char *target, const char *source, unsigned maxlen)
{
const char *p;
unsigned len;
if (target == NULL || maxlen == 0 || source == NULL)
{
return 0;
}
if ((p = (const char*)memchr(source, 0, maxlen))) /* djb-rwth: addressing LLVM warning */
{ /* maxlen does not include the found zero termination */
len = (int) (p - source);
}
else
{ /* reduced length does not include one more byte for zero termination */
len = maxlen - 1;
}
if (len)
{
memmove(target, source, len);
}
memset(target + len, 0, maxlen - len);
/* zero termination */ /* djb-rwth: memset_s C11/Annex K variant? */
return 1;
}

will read past the end of the source string when used like this:

mystrncpy( at[a1].elname, "H", sizeof( at->elname ) );

for this reason, I propose checking first if the source string is shorter than maxlen, and act appropriately

by the way, trying to be smarter than the standard library is rarely a good idea, I'd see if it's possible to just use strncpy or at leasemake mystrncpy a wrapper around it.

@djb-rwth djb-rwth self-assigned this Oct 21, 2025
@djb-rwth
Copy link
Collaborator

djb-rwth commented Oct 28, 2025

Hi @giallu,
Thanks for creating this PR.
The proposed fix should work.
This looks like a custom-made version of strncpy made by a previous developer with intention to overcome issues with zero termination.

It might be worth considering if target has been allocated with enough memory, but that would lead to writing custom boundary check function which would emulate strncpy_s; however, I definitely would not go that far.

@djb-rwth djb-rwth merged commit 9130397 into IUPAC-InChI:dev Oct 28, 2025
2 checks passed
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.

2 participants