Added more checking upon resolution and bytecode rewriting to verifier.
Access checks are performed when methods and fields are resolved. Also,
erroring bytecodes are now overwritten in the memory mapped dex file.
To do this, the code sets the memory mapped dex file as writable before
verification and set it back to read only after verification is done.
The overwritting occurs only in memory and the original dex file remains
unchanged.
Change-Id: I054394fb57e83d1ac5b6f200ab993d70cd9f55e6
diff --git a/src/compiler.cc b/src/compiler.cc
index 454c8f8..71727fd 100644
--- a/src/compiler.cc
+++ b/src/compiler.cc
@@ -143,6 +143,7 @@
}
void Compiler::VerifyDexFile(const ClassLoader* class_loader, const DexFile& dex_file) {
+ dex_file.ChangePermissions(PROT_READ | PROT_WRITE);
ClassLinker* class_linker = Runtime::Current()->GetClassLinker();
for (size_t class_def_index = 0; class_def_index < dex_file.NumClassDefs(); class_def_index++) {
const DexFile::ClassDef& class_def = dex_file.GetClassDef(class_def_index);
@@ -153,6 +154,7 @@
class_linker->VerifyClass(klass);
CHECK(klass->IsVerified() || klass->IsErroneous());
}
+ dex_file.ChangePermissions(PROT_READ);
}
void Compiler::InitializeClassesWithoutClinit(const ClassLoader* class_loader) {
diff --git a/src/dex_file.cc b/src/dex_file.cc
index 7c54018..924953f 100644
--- a/src/dex_file.cc
+++ b/src/dex_file.cc
@@ -70,6 +70,10 @@
}
}
+void DexFile::ChangePermissions(int prot) const {
+ closer_->ChangePermissions(prot);
+}
+
DexFile::Closer::~Closer() {}
DexFile::MmapCloser::MmapCloser(void* addr, size_t length) : addr_(addr), length_(length) {
@@ -80,9 +84,15 @@
PLOG(INFO) << "munmap failed";
}
}
+void DexFile::MmapCloser::ChangePermissions(int prot) {
+ if (mprotect(addr_, length_, prot) != 0) {
+ PLOG(FATAL) << "Failed to change dex file permissions to " << prot;
+ }
+}
DexFile::PtrCloser::PtrCloser(byte* addr) : addr_(addr) {}
DexFile::PtrCloser::~PtrCloser() { delete[] addr_; }
+void DexFile::PtrCloser::ChangePermissions(int prot) {}
const DexFile* DexFile::OpenFile(const std::string& filename,
const std::string& original_location,
@@ -106,7 +116,7 @@
return NULL;
}
size_t length = sbuf.st_size;
- void* addr = mmap(NULL, length, PROT_READ, MAP_SHARED, fd, 0);
+ void* addr = mmap(NULL, length, PROT_READ, MAP_PRIVATE, fd, 0);
if (addr == MAP_FAILED) {
PLOG(ERROR) << "mmap \"" << filename << "\" failed";
close(fd);
diff --git a/src/dex_file.h b/src/dex_file.h
index 3a9d58a..9ebcce2 100644
--- a/src/dex_file.h
+++ b/src/dex_file.h
@@ -832,11 +832,14 @@
}
}
+ void ChangePermissions(int prot) const;
+
private:
// Helper class to deallocate underlying storage.
class Closer {
public:
virtual ~Closer();
+ virtual void ChangePermissions(int prot) = 0;
};
// Helper class to deallocate mmap-backed .dex files.
@@ -844,6 +847,7 @@
public:
MmapCloser(void* addr, size_t length);
virtual ~MmapCloser();
+ virtual void ChangePermissions(int prot);
private:
void* addr_;
size_t length_;
@@ -854,6 +858,7 @@
public:
PtrCloser(byte* addr);
virtual ~PtrCloser();
+ virtual void ChangePermissions(int prot);
private:
byte* addr_;
};
diff --git a/src/dex_instruction.h b/src/dex_instruction.h
index 3934d7d..e08a1f7 100644
--- a/src/dex_instruction.h
+++ b/src/dex_instruction.h
@@ -32,6 +32,7 @@
k11n, // op vA, #+B
k11x, // op vAA
k10t, // op +AA
+ k20bc, // op AA, kind@BBBB
k20t, // op +AAAA
k22x, // op vAA, vBBBB
k21t, // op vAA, +BBBB
diff --git a/src/dex_instruction_list.h b/src/dex_instruction_list.h
index 009573e..6dc5def 100644
--- a/src/dex_instruction_list.h
+++ b/src/dex_instruction_list.h
@@ -238,7 +238,7 @@
V(0xEA, UNUSED_EA, "unused-ea", k10x, false, kUnknown, 0, kVerifyError) \
V(0xEB, UNUSED_EB, "unused-eb", k10x, false, kUnknown, 0, kVerifyError) \
V(0xEC, UNUSED_EC, "unused-ec", k10x, false, kUnknown, 0, kVerifyError) \
- V(0xED, UNUSED_ED, "unused-ed", k10x, false, kUnknown, 0, kVerifyError) \
+ V(0xED, THROW_VERIFICATION_ERROR, "throw-verification-error", k20bc, false, kNone, kThrow, kVerifyNone) \
V(0xEE, UNUSED_EE, "unused-ee", k10x, false, kUnknown, 0, kVerifyError) \
V(0xEF, UNUSED_EF, "unused-ef", k10x, false, kUnknown, 0, kVerifyError) \
V(0xF0, UNUSED_F0, "unused-f0", k10x, false, kUnknown, 0, kVerifyError) \
@@ -264,6 +264,7 @@
V(k11n) \
V(k11x) \
V(k10t) \
+ V(k20bc) \
V(k20t) \
V(k22x) \
V(k21t) \
diff --git a/src/dex_verifier.cc b/src/dex_verifier.cc
index a800f20..57e27e3 100644
--- a/src/dex_verifier.cc
+++ b/src/dex_verifier.cc
@@ -5,6 +5,7 @@
#include <iostream>
#include "class_linker.h"
+#include "dex_cache.h"
#include "dex_file.h"
#include "dex_instruction.h"
#include "dex_instruction_visitor.h"
@@ -3222,13 +3223,13 @@
break;
case Instruction::SHR_INT_LIT8:
tmp_type = AdjustForRightShift(work_line, dec_insn.vB_, dec_insn.vC_,
- false, &failure);
+ false);
CheckLitop(work_line, &dec_insn, tmp_type, kRegTypeInteger, false,
&failure);
break;
case Instruction::USHR_INT_LIT8:
tmp_type = AdjustForRightShift(work_line, dec_insn.vB_, dec_insn.vC_,
- true, &failure);
+ true);
CheckLitop(work_line, &dec_insn, tmp_type, kRegTypeInteger, false,
&failure);
break;
@@ -3244,8 +3245,7 @@
* which don't generally appear during verification. Because it's
* inserted in the course of verification, we can expect to see it here.
*/
- //case Instruction::THROW_VERIFICATION_ERROR:
- case Instruction::UNUSED_ED:
+ case Instruction::THROW_VERIFICATION_ERROR:
break;
/*
@@ -3373,17 +3373,10 @@
<< (int) dec_insn.opcode_ << " at 0x" << insn_idx << std::dec;
return false;
} else {
- // TODO: CHECK IF THIS WILL WORK!
- /* ignore the failure and move on */
- LOG(ERROR) << "VFY: failing opcode 0x" << std::hex
- << (int) dec_insn.opcode_ << " at 0x" << insn_idx << std::dec;
- failure = VERIFY_ERROR_NONE;
-#if 0
/* replace opcode and continue on */
LOG(ERROR) << "VFY: replacing opcode 0x" << std::hex
<< (int) dec_insn.opcode_ << " at 0x" << insn_idx << std::dec;
- if (!ReplaceFailingInstruction(code_item, insn_flags, insn_idx, failure))
- {
+ if (!ReplaceFailingInstruction(code_item, insn_idx, failure)) {
LOG(ERROR) << "VFY: rejecting opcode 0x" << std::hex
<< (int) dec_insn.opcode_ << " at 0x" << insn_idx
<< std::dec;
@@ -3395,7 +3388,6 @@
/* continue on as if we just handled a throw-verification-error */
failure = VERIFY_ERROR_NONE;
opcode_flag = Instruction::kThrow;
-#endif
}
}
@@ -3595,7 +3587,7 @@
}
bool DexVerifier::ReplaceFailingInstruction(const DexFile::CodeItem* code_item,
- InsnFlags* insn_flags, int insn_idx, VerifyError failure) {
+ int insn_idx, VerifyError failure) {
const uint16_t* insns = code_item->insns_ + insn_idx;
const byte* ptr = reinterpret_cast<const byte*>(insns);
const Instruction* inst = Instruction::At(ptr);
@@ -3672,14 +3664,13 @@
DCHECK(inst->IsThrow());
/* write a NOP over the third code unit, if necessary */
- int width = InsnGetWidth(insn_flags, insn_idx);
+ int width = inst->Size();
switch (width) {
case 2:
/* nothing to do */
break;
case 3:
- // TODO: Add this functionality
- //UpdateCodeUnit(method, insns + 2, Instruction::NOP);
+ UpdateCodeUnit(insns + 2, Instruction::NOP);
break;
default:
/* whoops */
@@ -3688,17 +3679,18 @@
}
/* encode the opcode, with the failure code in the high byte */
- // TODO: REPLACE FAILING OPCODES
- //DCHECK(width == 2 || width == 3);
- //uint16_t new_val = Instruction::THROW_VERIFICATION_ERROR |
- //uint16_t new_val = Instruction::UNUSED_ED |
- //(failure << 8) | (ref_type << (8 + kVerifyErrorRefTypeShift));
- //UpdateCodeUnit(method, insns, new_val);
+ DCHECK(width == 2 || width == 3);
+ uint16_t new_val = Instruction::THROW_VERIFICATION_ERROR |
+ (failure << 8) | (ref_type << (8 + kVerifyErrorRefTypeShift));
+ UpdateCodeUnit(insns, new_val);
return true;
}
-/* Handle a monitor-enter instruction. */
+void DexVerifier::UpdateCodeUnit(const uint16_t* ptr, uint16_t new_val) {
+ *(uint16_t*) ptr = new_val;
+}
+
void DexVerifier::HandleMonitorEnter(RegisterLine* work_line, uint32_t reg_idx,
uint32_t insn_idx, VerifyError* failure) {
if (!RegTypeIsReference(GetRegisterType(work_line, reg_idx))) {
@@ -3726,7 +3718,6 @@
work_line->monitor_stack_[work_line->monitor_stack_top_++] = insn_idx;
}
-/* Handle a monitor-exit instruction. */
void DexVerifier::HandleMonitorExit(RegisterLine* work_line, uint32_t reg_idx,
uint32_t insn_idx, VerifyError* failure) {
if (!RegTypeIsReference(GetRegisterType(work_line, reg_idx))) {
@@ -3777,23 +3768,17 @@
Method* method = vdata->method_;
const DexFile* dex_file = vdata->dex_file_;
UninitInstanceMap* uninit_map = vdata->uninit_map_.get();
- ClassLinker* class_linker = Runtime::Current()->GetClassLinker();
- DexCache* dex_cache = method->GetDeclaringClass()->GetDexCache();
- const ClassLoader* class_loader =
- method->GetDeclaringClass()->GetClassLoader();
- Field* field = NULL;
- Class* obj_class;
bool must_be_local = false;
if (!RegTypeIsReference(obj_type)) {
LOG(ERROR) << "VFY: attempt to access field in non-reference type "
<< obj_type;
*failure = VERIFY_ERROR_GENERIC;
- return field;
+ return NULL;
}
- field = class_linker->ResolveField(*dex_file, field_idx, dex_cache,
- class_loader, false);
+ Field* field = ResolveFieldAndCheckAccess(dex_file, field_idx,
+ method->GetDeclaringClass(), failure, false);
if (field == NULL) {
LOG(ERROR) << "VFY: unable to resolve instance field " << field_idx;
return field;
@@ -3807,7 +3792,7 @@
* the <init> method for the object and the field in question is
* declared by this class.
*/
- obj_class = RegTypeReferenceToClass(obj_type, uninit_map);
+ Class* obj_class = RegTypeReferenceToClass(obj_type, uninit_map);
DCHECK(obj_class != NULL);
if (RegTypeIsUninitReference(obj_type)) {
if (!IsInitMethod(method) || method->GetDeclaringClass() != obj_class) {
@@ -3849,21 +3834,14 @@
VerifyError* failure) {
Method* method = vdata->method_;
const DexFile* dex_file = vdata->dex_file_;
- ClassLinker* class_linker = Runtime::Current()->GetClassLinker();
- DexCache* dex_cache = method->GetDeclaringClass()->GetDexCache();
- const ClassLoader* class_loader =
- method->GetDeclaringClass()->GetClassLoader();
- Field* field;
-
- field = class_linker->ResolveField(*dex_file, field_idx, dex_cache,
- class_loader, true);
+ Field* field = ResolveFieldAndCheckAccess(dex_file, field_idx,
+ method->GetDeclaringClass(), failure, true);
if (field == NULL) {
- //const DexFile::FieldId field_id = dex_file->GetFieldId(field_idx);
-
- //LOG(ERROR) << "VFY: unable to resolve static field " << field_idx << " ("
- //<< dex_file->GetFieldName(field_id) << ") in "
- //<< dex_file->GetFieldClassDescriptor(field_id);
- LOG(ERROR) << "VFY: unable to resolve static field";
+ const DexFile::FieldId& field_id = dex_file->GetFieldId(field_idx);
+ LOG(ERROR) << "VFY: unable to resolve static field " << field_idx << " ("
+ << dex_file->GetFieldName(field_id) << ") in "
+ << dex_file->GetFieldClassDescriptor(field_id);
+ *failure = VERIFY_ERROR_NO_FIELD;
}
return field;
@@ -4349,7 +4327,7 @@
}
Class* DexVerifier::ResolveClassAndCheckAccess(const DexFile* dex_file,
- uint32_t class_idx, const Class* referrer, VerifyError* failure) {
+ uint32_t class_idx, const Class* referrer, VerifyError* failure) {
ClassLinker* class_linker = Runtime::Current()->GetClassLinker();
Class* res_class = class_linker->ResolveType(*dex_file, class_idx, referrer);
@@ -4370,6 +4348,103 @@
return res_class;
}
+Method* DexVerifier::ResolveMethodAndCheckAccess(const DexFile* dex_file,
+ uint32_t method_idx, const Class* referrer, VerifyError* failure,
+ bool is_direct) {
+ DexCache* dex_cache = referrer->GetDexCache();
+ Method* res_method = dex_cache->GetResolvedMethod(method_idx);
+
+ if (res_method == NULL) {
+ const DexFile::MethodId& method_id = dex_file->GetMethodId(method_idx);
+ Class* klass = ResolveClassAndCheckAccess(dex_file, method_id.class_idx_, referrer, failure);
+ if (klass == NULL) {
+ DCHECK(*failure != VERIFY_ERROR_NONE);
+ return NULL;
+ }
+
+ const char* name = dex_file->dexStringById(method_id.name_idx_);
+ std::string signature(dex_file->CreateMethodDescriptor(method_id.proto_idx_, NULL));
+ if (is_direct) {
+ res_method = klass->FindDirectMethod(name, signature);
+ } else if (klass->IsInterface()) {
+ res_method = klass->FindInterfaceMethod(name, signature);
+ } else {
+ res_method = klass->FindVirtualMethod(name, signature);
+ }
+ if (res_method != NULL) {
+ dex_cache->SetResolvedMethod(method_idx, res_method);
+ } else {
+ LOG(ERROR) << "VFY: couldn't find method "
+ << klass->GetDescriptor()->ToModifiedUtf8() << "." << name
+ << " " << signature;
+ *failure = VERIFY_ERROR_NO_METHOD;
+ return NULL;
+ }
+ }
+
+ /* Check if access is allowed. */
+ if (!referrer->CanAccess(res_method->GetDeclaringClass())) {
+ LOG(ERROR) << "VFY: illegal method access (call "
+ << res_method->GetDeclaringClass()->GetDescriptor()->ToModifiedUtf8()
+ << "." << res_method->GetName() << " "
+ << res_method->GetSignature() << " from "
+ << referrer->GetDescriptor()->ToModifiedUtf8() << ")";
+ *failure = VERIFY_ERROR_ACCESS_METHOD;
+ return NULL;
+ }
+
+ return res_method;
+}
+
+Field* DexVerifier::ResolveFieldAndCheckAccess(const DexFile* dex_file,
+ uint32_t field_idx, const Class* referrer, VerifyError* failure,
+ bool is_static) {
+ DexCache* dex_cache = referrer->GetDexCache();
+ Field* res_field = dex_cache->GetResolvedField(field_idx);
+
+ if (res_field == NULL) {
+ const DexFile::FieldId& field_id = dex_file->GetFieldId(field_idx);
+ Class* klass = ResolveClassAndCheckAccess(dex_file, field_id.class_idx_, referrer, failure);
+ if (klass == NULL) {
+ DCHECK(*failure != VERIFY_ERROR_NONE);
+ return NULL;
+ }
+
+ Class* field_type = ResolveClassAndCheckAccess(dex_file, field_id.type_idx_, referrer, failure);
+ if (field_type == NULL) {
+ DCHECK(*failure != VERIFY_ERROR_NONE);
+ return NULL;
+ }
+
+ const char* name = dex_file->dexStringById(field_id.name_idx_);
+ if (is_static) {
+ res_field = klass->FindStaticField(name, field_type);
+ } else {
+ res_field = klass->FindInstanceField(name, field_type);
+ }
+ if (res_field != NULL) {
+ dex_cache->SetResolvedfield(field_idx, res_field);
+ } else {
+ LOG(ERROR) << "VFY: couldn't find field "
+ << klass->GetDescriptor()->ToModifiedUtf8() << "." << name;
+ *failure = VERIFY_ERROR_NO_FIELD;
+ return NULL;
+ }
+ }
+
+ /* Check if access is allowed. */
+ if (!referrer->CanAccess(res_field->GetDeclaringClass())) {
+ LOG(ERROR) << "VFY: access denied from "
+ << referrer->GetDescriptor()->ToModifiedUtf8() << " to field "
+ << res_field->GetDeclaringClass()->GetDescriptor()->ToModifiedUtf8()
+ << "." << res_field->GetName()->ToModifiedUtf8();
+ *failure = VERIFY_ERROR_ACCESS_FIELD;
+ return NULL;
+ }
+
+ return res_field;
+}
+
DexVerifier::RegType DexVerifier::MergeTypes(RegType type1, RegType type2,
bool* changed) {
RegType result;
@@ -4769,7 +4844,7 @@
DexVerifier::RegType DexVerifier::AdjustForRightShift(
RegisterLine* register_line, int reg, unsigned int shift_count,
- bool is_unsigned_shift, VerifyError* failure) {
+ bool is_unsigned_shift) {
RegType src_type = GetRegisterType(register_line, reg);
RegType new_type;
@@ -4906,10 +4981,6 @@
const DexFile* dex_file = vdata->dex_file_;
const DexFile::CodeItem* code_item = vdata->code_item_;
UninitInstanceMap* uninit_map = vdata->uninit_map_.get();
- ClassLinker* class_linker = Runtime::Current()->GetClassLinker();
- DexCache* dex_cache = method->GetDeclaringClass()->GetDexCache();
- const ClassLoader* class_loader =
- method->GetDeclaringClass()->GetClassLoader();
Method* res_method;
std::string sig;
@@ -4921,8 +4992,8 @@
* Resolve the method. This could be an abstract or concrete method
* depending on what sort of call we're making.
*/
- res_method = class_linker->ResolveMethod(*dex_file, dec_insn->vB_, dex_cache,
- class_loader, (method_type == METHOD_DIRECT || method_type == METHOD_STATIC));
+ res_method = ResolveMethodAndCheckAccess(dex_file, dec_insn->vB_, method->GetDeclaringClass(),
+ failure, (method_type == METHOD_DIRECT || method_type == METHOD_STATIC));
if (res_method == NULL) {
const DexFile::MethodId& method_id = dex_file->GetMethodId(dec_insn->vB_);
diff --git a/src/dex_verifier.h b/src/dex_verifier.h
index 0170ebe..8b1f450 100644
--- a/src/dex_verifier.h
+++ b/src/dex_verifier.h
@@ -1147,7 +1147,10 @@
* Returns "true" on success.
*/
static bool ReplaceFailingInstruction(const DexFile::CodeItem* code_item,
- InsnFlags* insn_flags, int insn_idx, VerifyError failure);
+ int insn_idx, VerifyError failure);
+
+ /* Update a 16-bit opcode in a dex file. */
+ static void UpdateCodeUnit(const uint16_t* ptr, uint16_t new_val);
/* Handle a monitor-enter instruction. */
static void HandleMonitorEnter(RegisterLine* work_line, uint32_t reg_idx,
@@ -1401,6 +1404,30 @@
uint32_t class_idx, const Class* referrer, VerifyError* failure);
/*
+ * Resolves a method based on an index and performs access checks to ensure
+ * the referrer can access the resolved method.
+ *
+ * Does not throw exceptions.
+ *
+ * Sets "*failure" on failure.
+ */
+ static Method* ResolveMethodAndCheckAccess(const DexFile* dex_file,
+ uint32_t method_idx, const Class* referrer, VerifyError* failure,
+ bool is_direct);
+
+ /*
+ * Resolves a field based on an index and performs access checks to ensure
+ * the referrer can access the resolved field.
+ *
+ * Exceptions caused by failures are cleared before returning.
+ *
+ * Sets "*failure" on failure.
+ */
+ static Field* ResolveFieldAndCheckAccess(const DexFile* dex_file,
+ uint32_t class_idx, const Class* referrer, VerifyError* failure,
+ bool is_static);
+
+ /*
* Merge two RegType values.
*
* Sets "*changed" to "true" if the result doesn't match "type1".
@@ -1597,7 +1624,7 @@
* Returns the new register type.
*/
static RegType AdjustForRightShift(RegisterLine* register_line, int reg,
- unsigned int shift_count, bool is_unsigned_shift, VerifyError* failure);
+ unsigned int shift_count, bool is_unsigned_shift);
/*
* We're performing an operation like "and-int/2addr" that can be