littlefs-project / littlefs

A little fail-safe filesystem designed for microcontrollers
BSD 3-Clause "New" or "Revised" License
5.2k stars 797 forks source link

Incompatibility with clang-analyzer-core #365

Open ahollist opened 4 years ago

ahollist commented 4 years ago

When adding littlefs to a project that uses Clang static checking that includes clang-analyzer-core, the littlefs.c.o object throws several errors during compilation:

[6/27] Building C object third-party/CMakeFiles/littlefs.dir/littlefs/lfs.c.o
FAILED: third-party/CMakeFiles/littlefs.dir/littlefs/lfs.c.o
/usr/bin/cmake -E __run_co_compile --tidy="/usr/bin/clang-tidy-6.0;-format-style=file" --source=../third-party/littlefs/lfs.c -- /usr/bin/gcc -DBUILD_TEST  -O3 -DNDEBUG   -Wall -Werror -g -Wextra -Wshadow -Wundef -Wno-overflow -Wno-implicit-function-declaration -Wno-missing-field-initializers -std=gnu99 -MD -MT third-party/CMakeFiles/littlefs.dir/littlefs/lfs.c.o -MF third-party/CMakeFiles/littlefs.dir/littlefs/lfs.c.o.d -o third-party/CMakeFiles/littlefs.dir/littlefs/lfs.c.o   -c ../third-party/littlefs/lfs.c
/plantiga/build/../third-party/littlefs/lfs.c:367:18: error: 1st function call argument is an uninitialized value [clang-analyzer-core.CallAndMessage,-warnings-as-errors]
    a->tag     = lfs_fromle32(a->tag);
/**/build/../third-party/littlefs/lfs.c:1011:9: error: Call to function 'strcpy' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcpy'. CWE-119 [clang-analyzer-security.insecureAPI.strcpy,-warnings-as-errors]
        strcpy(info->name, "/");
        ^
/**/build/../third-party/littlefs/lfs.c:1011:9: note: Call to function 'strcpy' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcpy'. CWE-119
/**/build/../third-party/littlefs/lfs.c:2018:9: error: Call to function 'strcpy' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcpy'. CWE-119 [clang-analyzer-security.insecureAPI.strcpy,-warnings-as-errors]
        strcpy(info->name, ".");

Suppressing clang checking/warnings-as-errors on the library allows littlefs to build just fine, and indeed I can cross compile with arm-gcc with no problem.

I understand the reasons for using strcpy on such single character literals, but for the purposes of projects which may use clang or other static analysis, would it make sense to change these function calls to be the 'safer' strncpy or strlcpy?

Here are the resulting lines in my CMakeLists.txt that exclude Clang: (other compilation flags are set elsewhere)

###############################################################################
#### littlefs

set(_lfs_target littlefs)

set(_lfs_src
    littlefs/lfs.c
    littlefs/lfs_util.c
    )

add_library(${_lfs_target} ${_lfs_src})
target_include_directories(${_lfs_target}
    INTERFACE
        littlefs
)

set_property(TARGET ${_lfs_target} PROPERTY C_STANDARD 99)
# There are issues with our CLANG-TIDY and this library, we're going to suppress them for now
set_target_properties(${_lfs_target}
    PROPERTIES
        C_CLANG_TIDY ""
)
geky commented 4 years ago

Ah! Good find. I should see if I can add clang-analyzer-core to littlefs's CI.

I don't have anything against changing these to strncpy. Unfortunately it looks like strlcpy is not available in C99, which is a shame since it looks like it fixes the main flaw with strncpy (which IMO kinda defeats the main purpose of strncpy).