HomeFreeBSD

Fix unsafe string operations

Description

Fix unsafe string operations

Coverity caught unsafe use of strcpy() in ztest_dmu_objset_own(),
nfs_init_tmpfile() and dump_snapshot(). It also caught an unsafe use
of strlcat() in nfs_init_tmpfile().

Inspired by this, I did an audit of every single usage of strcpy() and
strcat() in the code. If I could not prove that the usage was safe, I
changed the code to use either strlcpy() or strlcat(), depending on
which function was originally used. In some cases, snprintf() was used
to replace multiple uses of strcat because it was cleaner.

Whenever I changed a function, I preferred to use sizeof(dst) when the
compiler is able to provide the string size via that. When it could not
because the string was passed by a caller, I checked the entire call
tree of the function to find out how big the buffer was and hard coded
it. Hardcoding is less than ideal, but it is safe unless someone shrinks
the buffer sizes being passed.

Additionally, Coverity reported three more string related issues:

  • It caught a case where we do an overlapping memory copy in a call to snprintf(). We fix that via kmem_strdup() and kmem_strfree().
  • It caught sizeof (buf) being used instead of buflen in zdb_nicenum()'s call to zfs_nicenum(), which is passed to snprintf(). We change that to pass buflen.
  • It caught a theoretical unterminated string passed to strcmp(). This one is likely a false positive, but we have the information needed to do this more safely, so we change this to silence the false positive not just in coverity, but potentially other static analysis tools too. We switch to strncmp().
  • There was a false positive in tests/zfs-tests/cmd/dir_rd_update.c. We suppress it by switching to snprintf() since other static analysis tools might complain about it too. Interestingly, there is a possible real bug there too, since it assumes that the passed directory path ends with '/'. We add a '/' to fix that potential bug.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Closes #13913

Details

Provenance
Richard Yao <richard.yao@alumni.stonybrook.edu>Authored on Sep 27 2022, 11:47 PM
GitHub <noreply@github.com>Committed on Sep 27 2022, 11:47 PM
Parents
rG88b199c24e78: Cleanup spa_export_common()
Branches
Unknown
Tags
Unknown