Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit d29c3e5a authored by Christopher Ferris's avatar Christopher Ferris Committed by Automerger Merge Worker
Browse files

Merge "Do not encode newline characters for abort/log." into main am: 033e4cee 

Original change: https://android-review.googlesource.com/c/platform/system/core/+/3395661



Change-Id: I737343f14bd7b08279004e12e5dba1480d55201b
Signed-off-by: default avatarAutomerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
parents 21255ee1 033e4cee

debuggerd/debuggerd_test.cpp

+68 −9
Original line number Original line Diff line number Diff line
@@ -3303,8 +3303,44 @@ TEST_F(CrasherTest, log_with_newline) { 
  ASSERT_MATCH(result, ":\\s*This is on the next line.");

  ASSERT_MATCH(result, ":\\s*This is on the next line.");

}

}





TEST_F(CrasherTest, log_with_non_utf8) {

TEST_F(CrasherTest, log_with_non_printable_ascii_verify_encoded) {

  StartProcess([]() { LOG(FATAL) << "Invalid UTF-8: \xA0\xB0\xC0\xD0 and some other data."; });

  static const std::string kEncodedStr =

      "\x5C\x31"

      "\x5C\x32"

      "\x5C\x33"

      "\x5C\x34"

      "\x5C\x35"

      "\x5C\x36"

      "\x5C\x37"

      "\x5C\x31\x30"

      "\x5C\x31\x36"

      "\x5C\x31\x37"

      "\x5C\x32\x30"

      "\x5C\x32\x31"

      "\x5C\x32\x32"

      "\x5C\x32\x33"

      "\x5C\x32\x34"

      "\x5C\x32\x35"

      "\x5C\x32\x36"

      "\x5C\x32\x37"

      "\x5C\x33\x30"

      "\x5C\x33\x31"

      "\x5C\x33\x32"

      "\x5C\x33\x33"

      "\x5C\x33\x34"

      "\x5C\x33\x35"

      "\x5C\x33\x36"

      "\x5C\x33\x37"

      "\x5C\x31\x37\x37"

      "\x5C\x32\x34\x30"

      "\x5C\x32\x36\x30"

      "\x5C\x33\x30\x30"

      "\x5C\x33\x32\x30";

  StartProcess([]() {

    LOG(FATAL) << "Encoded: "

                  "\x1\x2\x3\x4\x5\x6\x7\x8\xe\xf\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b"

                  "\x1c\x1d\x1e\x1f\x7f\xA0\xB0\xC0\xD0 after";

  });





  unique_fd output_fd;

  unique_fd output_fd;

  StartIntercept(&output_fd);

  StartIntercept(&output_fd);
@@ -3317,15 +3353,38 @@ TEST_F(CrasherTest, log_with_non_utf8) { 
  std::string result;

  std::string result;

  ConsumeFd(std::move(output_fd), &result);

  ConsumeFd(std::move(output_fd), &result);

  // Verify the abort message is sanitized properly.

  // Verify the abort message is sanitized properly.

  size_t pos = result.find(

  size_t pos = result.find(std::string("Abort message: 'Encoded: ") + kEncodedStr + " after'");

      "Abort message: 'Invalid UTF-8: "

      "\x5C\x32\x34\x30\x5C\x32\x36\x30\x5C\x33\x30\x30\x5C\x33\x32\x30 and some other data.'");

  EXPECT_TRUE(pos != std::string::npos) << "Couldn't find sanitized abort message: " << result;

  EXPECT_TRUE(pos != std::string::npos) << "Couldn't find sanitized abort message: " << result;





  // Make sure that the log message is sanitized properly too.

  // Make sure that the log message is sanitized properly too.

  EXPECT_TRUE(

  EXPECT_TRUE(result.find(std::string("Encoded: ") + kEncodedStr + " after", pos + 1) !=

      result.find("Invalid UTF-8: \x5C\x32\x34\x30\x5C\x32\x36\x30\x5C\x33\x30\x30\x5C\x33\x32\x30 "

              std::string::npos)

                  "and some other data.",

      << "Couldn't find sanitized log message: " << result;

                  pos + 30) != std::string::npos)

}



TEST_F(CrasherTest, log_with_with_special_printable_ascii) {

  static const std::string kMsg = "Not encoded: \t\v\f\r\n after";

  StartProcess([]() { LOG(FATAL) << kMsg; });



  unique_fd output_fd;

  StartIntercept(&output_fd);

  FinishCrasher();

  AssertDeath(SIGABRT);

  int intercept_result;

  FinishIntercept(&intercept_result);

  ASSERT_EQ(1, intercept_result) << "tombstoned reported failure";



  std::string result;

  ConsumeFd(std::move(output_fd), &result);

  // Verify the abort message does not remove characters that are UTF8 but

  // are, technically, not printable.

  size_t pos = result.find(std::string("Abort message: '") + kMsg + "'");

  EXPECT_TRUE(pos != std::string::npos) << "Couldn't find abort message: " << result;



  // Make sure that the log message is handled properly too.

  // The logger automatically splits a newline message into two pieces.

  pos = result.find("Not encoded: \t\v\f\r", pos + kMsg.size());

  EXPECT_TRUE(pos != std::string::npos) << "Couldn't find log message: " << result;

  EXPECT_TRUE(result.find(" after", pos + 1) != std::string::npos)

      << "Couldn't find sanitized log message: " << result;

      << "Couldn't find sanitized log message: " << result;

}
 
}

debuggerd/libdebuggerd/include/libdebuggerd/utility_host.h

+4 −1
Original line number Original line Diff line number Diff line
@@ -30,4 +30,7 @@ constexpr size_t kTagGranuleSize = 16; 
constexpr size_t kNumTagColumns = 16;

constexpr size_t kNumTagColumns = 16;

constexpr size_t kNumTagRows = 16;

constexpr size_t kNumTagRows = 16;





std::string oct_encode(const std::string& data);
 
// Encode all non-ascii values and also ascii values that are not printable.

std::string oct_encode_non_ascii_printable(const std::string& data);

// Encode any value that fails isprint(), includes encoding chars like '\n' and '\t'.

std::string oct_encode_non_printable(const std::string& data);

debuggerd/libdebuggerd/tombstone_proto.cpp

+2 −2
Original line number Original line Diff line number Diff line
@@ -467,7 +467,7 @@ static void dump_abort_message(Tombstone* tombstone, 
  msg.resize(index);

  msg.resize(index);





  // Make sure only UTF8 characters are present since abort_message is a string.

  // Make sure only UTF8 characters are present since abort_message is a string.

  tombstone->set_abort_message(oct_encode(msg));

  tombstone->set_abort_message(oct_encode_non_ascii_printable(msg));

}

}





static void dump_open_fds(Tombstone* tombstone, const OpenFilesList* open_files) {

static void dump_open_fds(Tombstone* tombstone, const OpenFilesList* open_files) {
@@ -776,7 +776,7 @@ static void dump_log_file(Tombstone* tombstone, const char* logger, pid_t pid) { 
      log_msg->set_priority(prio);

      log_msg->set_priority(prio);

      log_msg->set_tag(tag);

      log_msg->set_tag(tag);

      // Make sure only UTF8 characters are present since message is a string.

      // Make sure only UTF8 characters are present since message is a string.

      log_msg->set_message(oct_encode(msg));

      log_msg->set_message(oct_encode_non_ascii_printable(msg));

    } while ((msg = nl));

    } while ((msg = nl));

  }

  }

  android_logger_list_free(logger_list);

  android_logger_list_free(logger_list);

debuggerd/libdebuggerd/tombstone_proto_to_text.cpp

+3 −2
Original line number Original line Diff line number Diff line
@@ -17,6 +17,7 @@ 
#include <libdebuggerd/tombstone_proto_to_text.h>

#include <libdebuggerd/tombstone_proto_to_text.h>

#include <libdebuggerd/utility_host.h>

#include <libdebuggerd/utility_host.h>





#include <ctype.h>

#include <inttypes.h>

#include <inttypes.h>





#include <algorithm>

#include <algorithm>
@@ -463,8 +464,8 @@ static void print_main_thread(CallbackType callback, SymbolizeCallbackType symbo 
  }

  }





  for (const auto& crash_detail : tombstone.crash_details()) {

  for (const auto& crash_detail : tombstone.crash_details()) {

    std::string oct_encoded_name = oct_encode(crash_detail.name());

    std::string oct_encoded_name = oct_encode_non_printable(crash_detail.name());

    std::string oct_encoded_data = oct_encode(crash_detail.data());

    std::string oct_encoded_data = oct_encode_non_printable(crash_detail.data());

    CBL("Extra crash detail: %s: '%s'", oct_encoded_name.c_str(), oct_encoded_data.c_str());

    CBL("Extra crash detail: %s: '%s'", oct_encoded_name.c_str(), oct_encoded_data.c_str());

  }

  }

debuggerd/libdebuggerd/utility_host.cpp

+13 −4
Original line number Original line Diff line number Diff line
@@ -16,6 +16,7 @@ 




#include "libdebuggerd/utility_host.h"

#include "libdebuggerd/utility_host.h"





#include <ctype.h>

#include <sys/prctl.h>

#include <sys/prctl.h>





#include <charconv>

#include <charconv>
@@ -102,23 +103,31 @@ std::string describe_pac_enabled_keys(long value) { 
  return describe_end(value, desc);

  return describe_end(value, desc);

}

}





std::string oct_encode(const std::string& data) {

static std::string oct_encode(const std::string& data, bool (*should_encode_func)(int)) {

  std::string oct_encoded;

  std::string oct_encoded;

  oct_encoded.reserve(data.size());

  oct_encoded.reserve(data.size());





  // N.B. the unsigned here is very important, otherwise e.g. \255 would render as

  // N.B. the unsigned here is very important, otherwise e.g. \255 would render as

  // \-123 (and overflow our buffer).

  // \-123 (and overflow our buffer).

  for (unsigned char c : data) {

  for (unsigned char c : data) {

    if (isprint(c)) {

    if (should_encode_func(c)) {

      oct_encoded += c;

    } else {

      std::string oct_digits("\\\0\0\0", 4);

      std::string oct_digits("\\\0\0\0", 4);

      // char is encodable in 3 oct digits

      // char is encodable in 3 oct digits

      static_assert(std::numeric_limits<unsigned char>::max() <= 8 * 8 * 8);

      static_assert(std::numeric_limits<unsigned char>::max() <= 8 * 8 * 8);

      auto [ptr, ec] = std::to_chars(oct_digits.data() + 1, oct_digits.data() + 4, c, 8);

      auto [ptr, ec] = std::to_chars(oct_digits.data() + 1, oct_digits.data() + 4, c, 8);

      oct_digits.resize(ptr - oct_digits.data());

      oct_digits.resize(ptr - oct_digits.data());

      oct_encoded += oct_digits;

      oct_encoded += oct_digits;

    } else {

      oct_encoded += c;

    }

    }

  }

  }

  return oct_encoded;

  return oct_encoded;

}

}



std::string oct_encode_non_ascii_printable(const std::string& data) {

  return oct_encode(data, [](int c) { return !isgraph(c) && !isspace(c); });

}



std::string oct_encode_non_printable(const std::string& data) {

  return oct_encode(data, [](int c) { return !isprint(c); });

}