#19 Check for safe use of `memcpy` before warnings

[Michael Clark]

In the nRF52 SDK, file components/libraries/crypto/backend/oberon/oberon_backend_chacha_poly_aead.c, for the line memcpy( p_ctx->key, p_key, sizeof(p_ctx->key) ) flawfinder reports:

  Does not check for buffer overflows when copying to destination (CWE-120).
  Make sure destination can always hold the source data.

Would it be reasonable to not throw this error when using memcpy with the pattern memcpy(a, b, sizeof(a))? In general I'd like to use flawfinder and memcpy without needing to explicitly ignore every use of memcpy in the code, when the uses are checked. I follow the policy that warnings are errors, and that my code should compile warning free.

[David A. Wheeler]

Yes, that sounds very reasonable. Do you want to propose the patch to do that?

[Michael Clark]

Used test vectors:

memcpy(p_ctx->key, p_key, sizeof(p_ctx->key)); // good
memcpy(&foo, p_key, sizeof(foo)); // good
memcpy(&foo, p_key, sizeof(p_key)); // fail
memcpy(&foo, p_key, len); // fail

Patch attached - apply w/git am 001-fix-19.patch.

[David A. Wheeler]

Overall I like the solution. I have two comments:

1. the proposed regular expression uses dot star. However, that is a greedy match, and if using a longer expression such as a for Loop it will match too much. I think [^()\n] followed by star might be better. If you think that's wrong, let me know.
2. I would really like additional tests in the current automated test suite to cover this (at least 1 that matches the special case and one that does not). I am a big believer in automated tests.

If you can address those two things, I will gladly merge the patch. Thank you so much for your time.

[Michael Clark]

I've cleaned up your 1. w/([^)\s]*), added more whitespace checks, etc. For 2. I added 4 cases including whitespace checking and corresponding updates to the unit test suite. I agree that unit tests are nice, which is why I provided you with test vectors.

[David A. Wheeler]

Excellent! Thanks very much. Merged. Let me know of any other great ideas like that one!