Add safe string copy primitives

Test: string_tests
Bug: 78198288
Change-Id: I711dc633ebbc87bc2fc564a2d75c2575d87934fd

diff --git a/audio_utils/include/audio_utils/string.h b/audio_utils/include/audio_utils/string.h
new file mode 100644 (file)
index 0000000..806fcf8
--- /dev/null
+++ b/audio_utils/include/audio_utils/string.h
@@ -0,0 +1,50 @@
+/*
+ * Copyright 2018 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef ANDROID_AUDIO_STRING_H
+#define ANDROID_AUDIO_STRING_H
+
+#include <string.h>
+
+/** similar to strlcpy but also zero fills to end of string buffer, ensures no data leak
+    in parceled data sent over binder.*/
+inline size_t audio_utils_strlcpy_zerofill(char *dst, const char *src, size_t dst_size) {
+    const size_t srclen = strlcpy(dst, src, dst_size);
+    const size_t srclen_with_zero = srclen + 1; /* include zero termination in length. */
+    if (srclen_with_zero < dst_size) {
+        const size_t num_zeroes = dst_size - srclen_with_zero;
+        memset(dst + srclen_with_zero, 0 /* value */, num_zeroes); /* clear remaining buffer */
+    }
+    return srclen;
+}
+
+#ifdef __cplusplus
+
+/** similar to audio_utils_strlcpy_zerofill for fixed size destination string. */
+template <size_t size>
+inline size_t audio_utils_strlcpy_zerofill(char (&dst)[size], const char *src) {
+    return audio_utils_strlcpy_zerofill(dst, src, size);
+}
+
+/** similar to strlcpy for fixed size destination string. */
+template <size_t size>
+inline size_t audio_utils_strlcpy(char (&dst)[size], const char *src) {
+    return strlcpy(dst, src, size);
+}
+
+#endif // __cplusplus
+
+#endif // !ANDROID_AUDIO_STRING_H

diff --git a/audio_utils/tests/Android.bp b/audio_utils/tests/Android.bp
index 8eec07c..8952046 100644 (file)
--- a/audio_utils/tests/Android.bp
+++ b/audio_utils/tests/Android.bp
@@ -213,3 +213,15 @@ cc_test {
         },
     }
 }
+
+cc_test {
+    name: "string_tests",
+    host_supported: false,
+
+    shared_libs: ["libaudioutils"],
+    srcs: ["string_tests.cpp"],
+    cflags: [
+        "-Wall",
+        "-Werror",
+    ],
+}

diff --git a/audio_utils/tests/build_and_run_all_unit_tests.sh b/audio_utils/tests/build_and_run_all_unit_tests.sh
index 46f1cd6..2cfff84 100755 (executable)
--- a/audio_utils/tests/build_and_run_all_unit_tests.sh
+++ b/audio_utils/tests/build_and_run_all_unit_tests.sh
@@ -31,6 +31,10 @@ echo "testing channels"
 adb push $OUT/data/nativetest/channels_tests/channels_tests /system/bin
 adb shell /system/bin/channels_tests
 
+echo "string test"
+adb push $OUT/data/nativetest/string_tests/string_tests /system/bin
+adb shell /system/bin/string_tests
+
 echo "benchmarking primitives"
 adb push $OUT/system/bin/primitives_benchmark /system/bin
 adb shell /system/bin/primitives_benchmark

diff --git a/audio_utils/tests/string_tests.cpp b/audio_utils/tests/string_tests.cpp
new file mode 100644 (file)
index 0000000..2977624
--- /dev/null
+++ b/audio_utils/tests/string_tests.cpp
@@ -0,0 +1,86 @@
+/*
+ * Copyright 2018 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+//#define LOG_NDEBUG 0
+#define LOG_TAG "audio_utils_string_tests"
+
+#include <audio_utils/string.h>
+#include <gtest/gtest.h>
+
+// fills the string buffer with a increasing ramp of values from start.
+template <size_t size>
+void fill(char (&s)[size], int start) {
+    for (size_t i = 0; i < size - 1; ++i) {
+        s[i] = start++;
+    }
+    s[size - 1] = 0;
+}
+
+// checks that the fill counts from start, as expected to actual chars,
+// whereupon the rest is expected to be zeroes.
+template <size_t size>
+void check(char (&s)[size], int start, size_t actual) {
+    size_t lim = std::min(actual, size);
+    size_t i = 0;
+
+    if (lim > 0) {
+        for (; i < lim - 1; ++i) {
+            EXPECT_EQ(start, s[i]);
+            ++start;
+        }
+    }
+    for (; i < size; ++i) {
+        EXPECT_EQ(0, s[i]);
+    }
+}
+
+TEST(audio_utils_string, check_zero_fill) {
+    // we use string arrays whose size is known by compiler, not vectors
+    constexpr size_t STRING_SIZE = 50;
+    union {
+        char dst[STRING_SIZE];
+        char dst_mirror[STRING_SIZE + 10]; // verifier that we don't overwrite
+    };
+    char over[sizeof(dst) + 5];
+    char under[sizeof(dst) - 5];
+
+    // fill with a value ramp
+    constexpr int DST_START = 1;
+    constexpr int OVER_START = 2;
+    constexpr int UNDER_START = 3;
+    fill(dst_mirror, DST_START);
+    fill(over, OVER_START);
+    fill(under, UNDER_START);
+
+    // union should overlay dst and dst_mirror.
+    dst[sizeof(dst) - 1] = 0;
+    check(dst, DST_START, sizeof(dst));
+    EXPECT_EQ(sizeof(dst) + DST_START, dst_mirror[sizeof(dst)]);
+
+    // make sure we truncate when copying a larger string.
+    audio_utils_strlcpy_zerofill(dst, over);
+    check(dst, OVER_START, sizeof(dst));
+
+    // check we didn't overwrite
+    EXPECT_EQ(sizeof(dst) + DST_START, dst_mirror[sizeof(dst)]);
+
+    // make sure we fill remaining buffer with zeros.
+    audio_utils_strlcpy_zerofill(dst, under);
+    check(dst, UNDER_START, sizeof(under));
+
+    // check we didn't overwrite
+    EXPECT_EQ(sizeof(dst) + DST_START, dst_mirror[sizeof(dst)]);
+}