No checks for buffer()

[RootUp]

Hi Team,

This issue was observed while performing source code analysis that : https://github.com/facebook/zstd/blob/dev/programs/fileio.c#L1023

Does not check for buffer overflows when copying to destination [MS-banned] (CWE-120). Consider using snprintf, strcpy_s, or strlcpy (warning: strncpy easily misused).

Please have a look. (Whitehat Report #175277903183026)

Cheers!

[terrelln]

It looks like this particular strcpy() and strcat() are safe, but it certainly doesn't hurt to use the safer functions.

[Cyan4973]

I suspect that the analysis tool generically complains about usage of strcat(). But in this case, input and output sizes are controlled beforehand. We have the size of both inputs, the resulting output size is known, buffer is sized accordingly. So I'm pretty sure the concatenation will fit. Hence it looks like a false positive.

That being said, I'm not opposed using another function, such as strncat(), which feels safer. It sounds good practice, although to be complete, we'll have to care about potential portability issues, which can be surprising sometimes. (I don't remember here, but it could be that strcat() was preferred over strncat() due to portability difficulties. We'll have to check)

[Cyan4973]

Fixed by @edenzik

[RootUp]

Thank you very much for the fix, I just wonder is their any thanks file on repo, or can I get some mention for this bug, would be great if possible.

Cheers

[Cyan4973]

The NEWS file will typically name contributors and reporters. It's updated at each release.

[RootUp]

The NEWS file will typically name contributors and reporters. It's updated at each release.

Hey @Cyan4973 I cant find my name in NEWS file tho :( Please suggest