Repository: incubator-impala Updated Branches: refs/heads/master d0152d424 -> 4c4235f4b
IMPALA-5123: Fix ASAN use after free in timezone_db The issue is that the string temporary returned by .string goes out of scope immediately after being created. Also, the API to mkstemp is unclear on whether it modifies the string in place. Just strdup() the c_str() to be safe - this is not performance critical code. Testing: ASAN build, running expr-test be test; ASAN fails before, passes after this change. Change-Id: I490f741403ea2004bc51394aa1251577337b1e1d Reviewed-on: http://gerrit.cloudera.org:8080/6503 Reviewed-by: Lars Volker <[email protected]> Tested-by: Impala Public Jenkins Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/36ead908 Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/36ead908 Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/36ead908 Branch: refs/heads/master Commit: 36ead908f88f3719b600e793ddcd9a4058d88440 Parents: d0152d4 Author: Zach Amsden <[email protected]> Authored: Tue Mar 28 22:12:29 2017 +0000 Committer: Impala Public Jenkins <[email protected]> Committed: Thu Mar 30 11:27:36 2017 +0000 ---------------------------------------------------------------------- be/src/exprs/timezone_db.cc | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/36ead908/be/src/exprs/timezone_db.cc ---------------------------------------------------------------------- diff --git a/be/src/exprs/timezone_db.cc b/be/src/exprs/timezone_db.cc index 110d23b..37c466a 100644 --- a/be/src/exprs/timezone_db.cc +++ b/be/src/exprs/timezone_db.cc @@ -671,30 +671,34 @@ Status TimezoneDatabase::Initialize() { // Create a temporary file and write the timezone information. The boost // interface only loads this format from a file. We abort the startup if // this initialization fails for some reason. - char *filestr = const_cast<char*>((boost::filesystem::path(FLAGS_local_library_dir) - / string("impala.tzdb.XXXXXXX")).string().c_str()); + string pathname = (boost::filesystem::path(FLAGS_local_library_dir) / + string("impala.tzdb.XXXXXXX")).string(); + // mkstemp operates in place, so we need a mutable array. + std::vector<char> filestr(pathname.c_str(), pathname.c_str() + pathname.size() + 1); FILE* file; int fd; - if ((fd = mkstemp(filestr)) == -1) { + if ((fd = mkstemp(filestr.data())) == -1) { return Status(Substitute("Could not create temporary timezone file: $0. Check that " - "the directory $1 is writable by the user running Impala.", filestr, + "the directory $1 is writable by the user running Impala.", filestr.data(), FLAGS_local_library_dir)); } - if ((file = fopen(filestr, "w")) == NULL) { - unlink(filestr); + if ((file = fopen(filestr.data(), "w")) == NULL) { + unlink(filestr.data()); close(fd); - return Status(Substitute("Could not open temporary timezone file: $0", filestr)); + return Status(Substitute("Could not open temporary timezone file: $0", + filestr.data())); } if (fputs(TIMEZONE_DATABASE_STR, file) == EOF) { - unlink(filestr); + unlink(filestr.data()); close(fd); fclose(file); - return Status(Substitute("Could not load temporary timezone file: $0", filestr)); + return Status(Substitute("Could not load temporary timezone file: $0", + filestr.data())); } fclose(file); - tz_database_.load_from_file(string(filestr)); + tz_database_.load_from_file(string(filestr.data())); tz_region_list_ = tz_database_.region_list(); - unlink(filestr); + unlink(filestr.data()); close(fd); return Status::OK(); }
