1

In my code, all calls to memset appear as warnings with the flawfinder tool.

In the simplest case it could boil down to the equivalent to

    float f1;
    float f2;
    void* p1 = &f1;
    void* p2 = &f2;
    memcpy(p1, p2, sizeof(float));

The message is

./file.cpp:10:  [2] (buffer) memcpy:
  Does not check for buffer overflows when copying to destination (CWE-120).
  Make sure destination can always hold the source data.

I absolutely understand that this could be replaced by a simple copy, this is just a simplified example. I also understand what are the potential problems with using memcpy and buffer overrun.

The question is what is exactly flawfinder asking me to do here?

Perhaps something like adding an assert? (this didn't suppress the warning)

    assert( sizeof(*p1) == sizeof(*p2) );
    memcpy(p1, p2, sizeof(float));

Or is it just telling me just don't use memset?

I am programming in C++, but I am pretty sure the question and the solution is common to both C and C++ languages.

alfC
  • 14,261
  • 4
  • 67
  • 118
  • If you program in C++ then please don't add the unrelated C language tag. C and C++ are two *very* different languages. – Some programmer dude Dec 03 '21 at 12:21
  • This CWE thing doesn't sound like a trustworthy coding standard. Can you tell the tool to check against CERT-C or MISRA-C instead? – Lundin Dec 03 '21 at 12:25
  • It seems [someone had a similar problem, but the community didn't find a solution](https://stackoverflow.com/questions/36307269/i-cant-solve-this-flawfinder-warning-cwe-78-cwe-120). There are some helpful comments there, though. – Leonardo Alves Machado Dec 03 '21 at 12:29
  • @Someprogrammerdude, the question applies to both languages, the tool is also for both languages. `memset` can be invoked from both languages. I can replace `std::memset` by `memset`. The idea of tags is that people can find answers to their questions, whether they program in C or C++. They will likely have the same answer if there is one. – alfC Dec 03 '21 at 12:41
  • @Lundin, just checked `flawfinder --listrules` and all (225) are CWE-XXX rules. I am just running as many static analysis tools as I can to a code. I use `clang-tidy` which seems to cover many CERT rules and `cppcheck` (seem to have its own rules). None of which complained about `memset`. If you know what tool to use to check for some MISRA-C rules, please let me know. – alfC Dec 03 '21 at 12:44
  • 3
    @alfC: The tool may be used for both languages and `memset` may be called from both, but that does not mean the question is the same for both languages. The solutions (or lack thereof) may be different in the different languages due to rules about conversions and other issues. Mixing the two tags makes it harder for people seeking C solutions to find them among C++ discussion and vice-versa. Do not tag both C and C++ except when asking about differences or interactions between the two languages. – Eric Postpischil Dec 03 '21 at 12:50
  • @EricPostpischil, thank you for the clarification. Once there is a solution and if the solution is also common, e.g. word by word to C and C++, would you tag it as C as well? – alfC Dec 03 '21 at 12:56
  • @Lundin, just checked https://cwe.mitre.org/data/definitions/120.html (i.e. CWE-120) and all the examples I see talk about null-terminated strings or fixed size buffers. No mention of `memset`. I think `flawfinder` may think that one is trying to copy null-terminad strings and pass beyond end problems. – alfC Dec 03 '21 at 13:13
  • "I am just running as many static analysis tools as I can to a code" Well, that's asking for a flood of false positives. If you do that, it should probably be in combination with code reviews. You either need to be a C or C++ expert or have one in your team which you can consult. Regarding MISRA-C and MISRA-C++, most tools for it are commercial. There is an open source static analyser called Frama C but I haven't used it. – Lundin Dec 03 '21 at 14:01
  • @Lundin, that is fine, one false positive at a time. Thank you for the information. I heard about MISRA before but after your comment I realized that none of the tools I used so far mentioned MISRA rules. I followed the links for Frama (ended up here https://www.frama-c.com/fc-plugins/frama-clang.html) but the tool doesn't compile in my system for some reason. (`zarith.cma: not found`, not even after installing `libfindocaml`). – alfC Dec 03 '21 at 20:34

1 Answers1

2
errno_t  err = memcpy_s(dest, dsize, src, cnt);

that should be the 'safe' version which hopefully satisfies flawfinder

alfC
  • 14,261
  • 4
  • 67
  • 118
Giel
  • 397
  • 1
  • 9
  • I didn't know about `memcpy_s`, I am trying to figure out the difference exactly. I guess the equivalent of `memcpy(p1, p2, sizeof(float));` is going to be `memcpy_s(p1, sizeof(float), p2, sizeof(float))`, or more symmetric `memcpy_s(f1, sizeof(*f1), f2, sizeof(*f2))`. This is the best description I found so far "These functions [ (w)memcpy_s] ] validate their parameters. If count is non-zero and dest or src is a null pointer, or destSize is smaller than count, these functions invoke the invalid parameter handler, as described in Parameter Validation." they return status as well. – alfC Dec 03 '21 at 21:47
  • Seems that some acrobatics is needed to use `memcpy_s` https://stackoverflow.com/questions/31278172/undefined-reference-to-memcpy-s. It seems that is not so easy from C++ (as @EricPostpischil predicted). In any case, looking at my code none of the checks that `memcpy_s` potentially are useful in my case because I am dealing with stack variables (that are never null) and have compile-time sizes (that I check separately with `static_assert( sizeof(t1) == sizeof(t2) );`. In summary, I think this could appease flawfinder but I was not able to verify it yet. – alfC Dec 03 '21 at 22:33
  • is there a reason you can't use assignment operators for it or another 'modern' concept? – Giel Dec 03 '21 at 22:41
  • good question, I have to revisit the logic but I think I was using `std::memcpy` instead of assignment through `reinterpret_cast` to avoid violation of aliasing rules. (Look for "memset" here https://en.cppreference.com/w/cpp/language/reinterpret_cast) – alfC Dec 03 '21 at 23:13