From c5e409aa6b5083b6a7a431ac920455ca41ed0db7 Mon Sep 17 00:00:00 2001 From: Andy Hung Date: Tue, 17 Apr 2018 18:13:05 -0700 Subject: [PATCH] Add safe string copy primitives Test: string_tests Bug: 78198288 Change-Id: I711dc633ebbc87bc2fc564a2d75c2575d87934fd --- audio_utils/include/audio_utils/string.h | 50 +++++++++++++ audio_utils/tests/Android.bp | 12 ++++ audio_utils/tests/build_and_run_all_unit_tests.sh | 4 ++ audio_utils/tests/string_tests.cpp | 86 +++++++++++++++++++++++ 4 files changed, 152 insertions(+) create mode 100644 audio_utils/include/audio_utils/string.h create mode 100644 audio_utils/tests/string_tests.cpp diff --git a/audio_utils/include/audio_utils/string.h b/audio_utils/include/audio_utils/string.h new file mode 100644 index 00000000..806fcf82 --- /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 + +/** 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 +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 +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 8eec07cb..89520461 100644 --- 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 46f1cd64..2cfff84a 100755 --- 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 index 00000000..29776244 --- /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 +#include + +// fills the string buffer with a increasing ramp of values from start. +template +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 +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)]); +} -- 2.11.0