libbrillo: blkdevutils: avoid passing non-null terminated data as C-string
SecureBlobs created from std::strings do not end in '\0'.
In this case, the dm code calls strlen() on the parameter string
which results in an access beyond the allocated space. This caused
the ASAN builder to fail continuously. Instead, for now, use the
string functions for SecureBlob to pass the data on.
BUG=chromium:941727
TEST=amd-generic vm boots up.
Change-Id: I6c63b37bae1204c9dca6315f2285f16f4405296a
Reviewed-on: https://chromium-review.googlesource.com/1524550
Commit-Ready: Sarthak Kukreti <sarthakkukreti@chromium.org>
Tested-by: Sarthak Kukreti <sarthakkukreti@chromium.org>
Reviewed-by: Manoj Gupta <manojgupta@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>
Cr-Mirrored-From: https://chromium.googlesource.com/chromiumos/platform2
Cr-Mirrored-Commit: 70164ed0d01338cff38777b0bbeb71a79ffe6b6c
diff --git a/brillo/blkdev_utils/device_mapper_task.cc b/brillo/blkdev_utils/device_mapper_task.cc
index 22d2455..f2cbadd 100644
--- a/brillo/blkdev_utils/device_mapper_task.cc
+++ b/brillo/blkdev_utils/device_mapper_task.cc
@@ -27,12 +27,21 @@
uint64_t length,
const std::string& type,
const SecureBlob& parameters) {
+ // Strings stored in SecureBlob don't end with '\0'. Unfortunately,
+ // this causes accesses beyond the allocated storage space if any
+ // of the functions expecting a c-string get passed a SecureBlob.data().
+ // Temporarily, assign to a string.
+ // TODO(sarthakkukreti): Evaluate creation of a SecureCString to keep
+ // string data safe.
+ std::string parameters_str = parameters.to_string();
if (!task_ ||
!dm_task_add_target(task_.get(), start, length, type.c_str(),
- reinterpret_cast<const char*>(parameters.data()))) {
+ parameters_str.c_str())) {
LOG(ERROR) << "AddTarget failed";
return false;
}
+ // Clear the string.
+ parameters_str.clear();
return true;
}
