Resolve compiler warnings (#597)

* Suppress false positive warning

Recent versions of GCC are getting smarter, but here it has outsmarted
itself. Drop the compiler a hint that 'possible null pointer dereference'
is in fact impossible in this case.

* Do not "fix up" iterator ordering

Recent versions of GCC started printing a warning about those lines [1]:

    if (begin < end) {
        return input_buffer(&*begin, end - begin);
    }

It says that we are "assuming pointer wraparound does not occur when
comparing P +- C1 with P +- C2". In fact, we do assume that.

[1]: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=49283

Here we try to be "helpful" and correct a possible mistake of swapping
begin and end iterators. However, after thinking about it, this mistake
is unlikely to happen. Moreover, we really rely on the user to provide
correct iterators:

 1) They should point into the same container.
 2) They should point to valid elements of said container.
 3) The container must store its elements in contiguous memory.
 4) The "begin" iterator should come before the "end" one.

If any of these assumptions is broken, you get undefined behavior (most
likely resuling in a crash caused by segmentation fault). We cannot
verify these assumptions by simply checking that one address is lower
than the other. Debug builds of STL may check that, but that's as good
as you can get it in C++.

So stop doing that (un)helpful fixup. Remove the check and do not swap
iterators, removing the cause of the warning too.

* Initialize structs with memset() manually

In C, it is perfectly legal to zero-initialize structures with {0},
like this:

    struct themis_scell_pbkdf2_context kdf = {0};

However, this is not (always) legal in C++. Certain versions of
afl-clang are more C++-leaning and produce warnings about uninitialized
fields even when compiling C code.

Well, okay, I don't want to fence this code with suppressions,
so replace this initialization is explicit memset() calls.

src/themis/secure_cell_seal_passphrase.c

@@ -65,8 +65,9 @@ static themis_status_t themis_auth_sym_derive_encryption_key_pbkdf2(
 {
     themis_status_t res = THEMIS_FAIL;
     uint8_t salt[THEMIS_AUTH_SYM_PBKDF2_SALT_LENGTH] = {0};
-    struct themis_scell_pbkdf2_context kdf = {0};
+    struct themis_scell_pbkdf2_context kdf;
 
+    memset(&kdf, 0, sizeof(kdf));
     kdf.iteration_count = THEMIS_AUTH_SYM_PBKDF2_ITERATIONS;
     kdf.salt = salt;
     kdf.salt_length = sizeof(salt);
@@ -157,13 +158,14 @@ themis_status_t themis_auth_sym_encrypt_message_with_passphrase_(const uint8_t*
     uint8_t auth_tag[THEMIS_AUTH_SYM_AUTH_TAG_LENGTH] = {0};
     uint8_t derived_key[THEMIS_AUTH_SYM_KEY_LENGTH / 8] = {0};
     size_t derived_key_length = sizeof(derived_key);
-    struct themis_scell_auth_token_passphrase hdr = {0};
+    struct themis_scell_auth_token_passphrase hdr;
 
     /* Message length is currently stored as 32-bit integer, sorry */
     if (message_length > UINT32_MAX) {
         return THEMIS_INVALID_PARAMETER;
     }
 
+    memset(&hdr, 0, sizeof(hdr));
     hdr.alg = THEMIS_AUTH_SYM_PASSPHRASE_ALG;
     hdr.iv = iv;
     hdr.iv_length = sizeof(iv);
@@ -271,8 +273,9 @@ static themis_status_t themis_auth_sym_derive_decryption_key_pbkdf2(
     size_t derived_key_length)
 {
     themis_status_t res = THEMIS_FAIL;
-    struct themis_scell_pbkdf2_context kdf = {0};
+    struct themis_scell_pbkdf2_context kdf;
 
+    memset(&kdf, 0, sizeof(kdf));
     res = themis_read_scell_pbkdf2_context(hdr, &kdf);
     if (res != THEMIS_SUCCESS) {
         return res;
@@ -331,11 +334,12 @@ themis_status_t themis_auth_sym_decrypt_message_with_passphrase_(const uint8_t*
                                                                  size_t* message_length)
 {
     themis_status_t res = THEMIS_FAIL;
-    struct themis_scell_auth_token_passphrase hdr = {0};
+    struct themis_scell_auth_token_passphrase hdr;
     /* Use maximum possible length, not the default one */
     uint8_t derived_key[THEMIS_AUTH_SYM_MAX_KEY_LENGTH / 8] = {0};
     size_t derived_key_length = sizeof(derived_key);
 
+    memset(&hdr, 0, sizeof(hdr));
     res = themis_read_scell_auth_token_passphrase(auth_token, auth_token_length, &hdr);
     if (res != THEMIS_SUCCESS) {
         return res;

src/wrappers/themis/themispp/impl/input_buffer.hpp

@@ -117,13 +117,10 @@ inline input_buffer input_bytes(Iterator begin, Iterator end) noexcept
     // are indeed contiguous, so we trust the application code with that.
     // The behavior is undefined (i.e., we'll crash) if memory between
     // begin and end is not addressable.
-    if (begin < end) {
-        return input_buffer(&*begin, end - begin);
-    }
     if (begin == end) {
         return input_buffer();
     }
-    return input_buffer(&*end, begin - end);
+    return input_buffer(&*begin, end - begin);
 }
 
 // STL-compatible contiguous containers

tests/themispp/input_buffer_test.hpp

@@ -85,10 +85,6 @@ static inline void input_buffer_bytes()
         std::vector<uint8_t> vector(5, 0);
         themispp::impl::input_buffer buf1 = themispp::impl::input_bytes(vector.begin(), vector.end());
         sput_fail_unless(buf1.data() == &vector[0] && buf1.size() == 5, "std::vector (iterators)", __LINE__);
-        themispp::impl::input_buffer buf2 = themispp::impl::input_bytes(vector.end(), vector.begin());
-        sput_fail_unless(buf2.data() == &vector[0] && buf2.size() == 5,
-                         "std::vector (iterators, flip)",
-                         __LINE__);
     }
     {
         uint8_t array[] = {1, 2, 3}; // NOLINT(cppcoreguidelines-avoid-c-arrays)

tests/themispp/secret_bytes_test.hpp

@@ -61,10 +61,15 @@ static inline void test_secret_bytes()
     other_vector.push_back(9);
     secret_bytes move_vector(std::move(other_vector));
     sput_fail_unless(move_vector.size() == 2, "move std::vector", __LINE__);
+    // GCC says "potential null pointer dereference" but that's false positive:
+    // "move_vector.data()" is never null because "move_vector.size() == 2".
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wnull-dereference"
     // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic)
     sput_fail_unless(move_vector.data()[0] == 8 && move_vector.data()[1] == 9,
                      "move std::vector: data",
                      __LINE__);
+#pragma GCC diagnostic pop
     // NOLINTNEXTLINE(bugprone-use-after-move)
     sput_fail_unless(other_vector.empty(), "move std::vector: moved", __LINE__);
 #endif