commit b0b0b496125c16620e99ea4e4a05693c281eccf1
author Sebastien Hertz <shertz@google.com> Mon Sep 15 11:27:27 2014 +0200
committer Sebastien Hertz <shertz@google.com> Wed Sep 17 11:52:24 2014 +0200
tree a73507873a28411f843b98701bcd0535d1dc361a
parent 97ca64b01e14de77ba14067b26069405d0dba0bf

Avoid crash in StringReference.Value JDWP command

Checks for null or invalid object id. Also checks whether the corresponding
object is a java.lang.String.

Bug: 17492221
Bug: 15005460

(cherry picked from commit 29259fa6b0514866d2d4bf57d58c1557b26abbb7)

Change-Id: I52673bdef6912a4cccf5a6eeecb6e1e817b9dd6b

runtime/debugger.cc

diff --git a/runtime/debugger.cc b/runtime/debugger.cc
index 001032c..df51973 100644
--- a/runtime/debugger.cc
+++ b/runtime/debugger.cc
@@ -1886,11 +1886,25 @@
   return SetFieldValueImpl(0, field_id, value, width, true);
 }
 
-std::string Dbg::StringToUtf8(JDWP::ObjectId string_id) {
+JDWP::JdwpError Dbg::StringToUtf8(JDWP::ObjectId string_id, std::string* str) {
   JDWP::JdwpError error;
-  mirror::String* s = gRegistry->Get<mirror::String*>(string_id, &error);
-  CHECK(s != nullptr) << error;
-  return s->ToModifiedUtf8();
+  mirror::Object* obj = gRegistry->Get<mirror::Object*>(string_id, &error);
+  if (error != JDWP::ERR_NONE) {
+    return error;
+  }
+  if (obj == nullptr) {
+    return JDWP::ERR_INVALID_OBJECT;
+  }
+  {
+    ScopedObjectAccessUnchecked soa(Thread::Current());
+    mirror::Class* java_lang_String = soa.Decode<mirror::Class*>(WellKnownClasses::java_lang_String);
+    if (!java_lang_String->IsAssignableFrom(obj->GetClass())) {
+      // This isn't a string.
+      return JDWP::ERR_INVALID_STRING;
+    }
+  }
+  *str = obj->AsString()->ToModifiedUtf8();
+  return JDWP::ERR_NONE;
 }
 
 void Dbg::OutputJValue(JDWP::JdwpTag tag, const JValue* return_value, JDWP::ExpandBuf* pReply) {

runtime/debugger.h

diff --git a/runtime/debugger.h b/runtime/debugger.h
index ab758ca..e171d78 100644
--- a/runtime/debugger.h
+++ b/runtime/debugger.h
@@ -381,7 +381,7 @@
   static JDWP::JdwpError SetStaticFieldValue(JDWP::FieldId field_id, uint64_t value, int width)
       SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
 
-  static std::string StringToUtf8(JDWP::ObjectId string_id)
+  static JDWP::JdwpError StringToUtf8(JDWP::ObjectId string_id, std::string* str)
       SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
   static void OutputJValue(JDWP::JdwpTag tag, const JValue* return_value, JDWP::ExpandBuf* pReply)
       SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);

runtime/jdwp/jdwp_handler.cc

diff --git a/runtime/jdwp/jdwp_handler.cc b/runtime/jdwp/jdwp_handler.cc
index 8560cb5..e0a83f6 100644
--- a/runtime/jdwp/jdwp_handler.cc
+++ b/runtime/jdwp/jdwp_handler.cc
@@ -151,7 +151,12 @@
     /* show detailed debug output */
     if (resultTag == JT_STRING && exceptObjId == 0) {
       if (resultValue != 0) {
-        VLOG(jdwp) << "      string '" << Dbg::StringToUtf8(resultValue) << "'";
+        if (VLOG_IS_ON(jdwp)) {
+          std::string result_string;
+          JDWP::JdwpError error = Dbg::StringToUtf8(resultValue, &result_string);
+          CHECK_EQ(error, JDWP::ERR_NONE);
+          VLOG(jdwp) << "      string '" << result_string << "'";
+        }
       } else {
         VLOG(jdwp) << "      string (null)";
       }
@@ -919,7 +924,11 @@
 static JdwpError SR_Value(JdwpState*, Request* request, ExpandBuf* pReply)
     SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
   ObjectId stringObject = request->ReadObjectId();
-  std::string str(Dbg::StringToUtf8(stringObject));
+  std::string str;
+  JDWP::JdwpError error = Dbg::StringToUtf8(stringObject, &str);
+  if (error != JDWP::ERR_NONE) {
+    return error;
+  }
 
   VLOG(jdwp) << StringPrintf("    --> %s", PrintableString(str.c_str()).c_str());