Fix: bytecode validator: reject specialized load field/context ref instructions

Reject specialized load ref and get context ref instructions so a
bytecode crafted with nefarious intent cannot read a memory area larger
than the memory targeted by the instrumentation.

This prevents bytecode received from the session daemon from performing
out of bound memory accesses and from disclosing the content of
application memory beyond what has been targeted by the instrumentation.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Change-Id: Ica16b97167d391d86e73b05fbf0210ff52b9c9f1

diff --git a/src/lib/lttng-ust/lttng-bytecode-validator.c b/src/lib/lttng-ust/lttng-bytecode-validator.c
index d727270ae7bcd10bc89eddfbbdacf1ff887cb8ec..e862d1596f3cd77a98cdba311ba5a81067cc4439 100644 (file)
--- a/src/lib/lttng-ust/lttng-bytecode-validator.c
+++ b/src/lib/lttng-ust/lttng-bytecode-validator.c
@@ -392,15 +392,8 @@ int bytecode_validate_overflow(struct bytecode_runtime *bytecode,
                break;
        }
 
-       /* load field ref */
+       /* load field and get context ref */
        case BYTECODE_OP_LOAD_FIELD_REF:
-       {
-               ERR("Unknown field ref type\n");
-               ret = -EINVAL;
-               break;
-       }
-
-       /* get context ref */
        case BYTECODE_OP_GET_CONTEXT_REF:
        case BYTECODE_OP_LOAD_FIELD_REF_STRING:
        case BYTECODE_OP_LOAD_FIELD_REF_SEQUENCE:
@@ -1217,6 +1210,236 @@ int validate_instruction_all_contexts(struct bytecode_runtime *bytecode,
        return 0;
 }
 
+/*
+ * Validate load instructions: specialized instructions not accepted as input.
+ *
+ * Return value:
+ * >0: going to next insn.
+ * 0: success, stop iteration.
+ * <0: error
+ */
+static
+int validate_load(char **_next_pc,
+               char *pc)
+{
+       int ret = 0;
+       char *next_pc = *_next_pc;
+
+       switch (*(bytecode_opcode_t *) pc) {
+       case BYTECODE_OP_UNKNOWN:
+       default:
+       {
+               ERR("Unknown bytecode op %u\n",
+                       (unsigned int) *(bytecode_opcode_t *) pc);
+               ret = -EINVAL;
+               goto end;
+       }
+
+       case BYTECODE_OP_RETURN:
+       {
+               next_pc += sizeof(struct return_op);
+               break;
+       }
+
+       case BYTECODE_OP_RETURN_S64:
+       {
+               next_pc += sizeof(struct return_op);
+               break;
+       }
+
+       /* binary */
+       case BYTECODE_OP_MUL:
+       case BYTECODE_OP_DIV:
+       case BYTECODE_OP_MOD:
+       case BYTECODE_OP_PLUS:
+       case BYTECODE_OP_MINUS:
+       {
+               ERR("Unsupported bytecode op %u\n",
+                       (unsigned int) *(bytecode_opcode_t *) pc);
+               ret = -EINVAL;
+               goto end;
+       }
+
+       case BYTECODE_OP_EQ:
+       case BYTECODE_OP_NE:
+       case BYTECODE_OP_GT:
+       case BYTECODE_OP_LT:
+       case BYTECODE_OP_GE:
+       case BYTECODE_OP_LE:
+       case BYTECODE_OP_EQ_STRING:
+       case BYTECODE_OP_NE_STRING:
+       case BYTECODE_OP_GT_STRING:
+       case BYTECODE_OP_LT_STRING:
+       case BYTECODE_OP_GE_STRING:
+       case BYTECODE_OP_LE_STRING:
+       case BYTECODE_OP_EQ_STAR_GLOB_STRING:
+       case BYTECODE_OP_NE_STAR_GLOB_STRING:
+       case BYTECODE_OP_EQ_S64:
+       case BYTECODE_OP_NE_S64:
+       case BYTECODE_OP_GT_S64:
+       case BYTECODE_OP_LT_S64:
+       case BYTECODE_OP_GE_S64:
+       case BYTECODE_OP_LE_S64:
+       case BYTECODE_OP_EQ_DOUBLE:
+       case BYTECODE_OP_NE_DOUBLE:
+       case BYTECODE_OP_GT_DOUBLE:
+       case BYTECODE_OP_LT_DOUBLE:
+       case BYTECODE_OP_GE_DOUBLE:
+       case BYTECODE_OP_LE_DOUBLE:
+       case BYTECODE_OP_EQ_DOUBLE_S64:
+       case BYTECODE_OP_NE_DOUBLE_S64:
+       case BYTECODE_OP_GT_DOUBLE_S64:
+       case BYTECODE_OP_LT_DOUBLE_S64:
+       case BYTECODE_OP_GE_DOUBLE_S64:
+       case BYTECODE_OP_LE_DOUBLE_S64:
+       case BYTECODE_OP_EQ_S64_DOUBLE:
+       case BYTECODE_OP_NE_S64_DOUBLE:
+       case BYTECODE_OP_GT_S64_DOUBLE:
+       case BYTECODE_OP_LT_S64_DOUBLE:
+       case BYTECODE_OP_GE_S64_DOUBLE:
+       case BYTECODE_OP_LE_S64_DOUBLE:
+       case BYTECODE_OP_BIT_RSHIFT:
+       case BYTECODE_OP_BIT_LSHIFT:
+       case BYTECODE_OP_BIT_AND:
+       case BYTECODE_OP_BIT_OR:
+       case BYTECODE_OP_BIT_XOR:
+       {
+               next_pc += sizeof(struct binary_op);
+               break;
+       }
+
+       /* unary */
+       case BYTECODE_OP_UNARY_PLUS:
+       case BYTECODE_OP_UNARY_MINUS:
+       case BYTECODE_OP_UNARY_PLUS_S64:
+       case BYTECODE_OP_UNARY_MINUS_S64:
+       case BYTECODE_OP_UNARY_NOT_S64:
+       case BYTECODE_OP_UNARY_NOT:
+       case BYTECODE_OP_UNARY_BIT_NOT:
+       case BYTECODE_OP_UNARY_PLUS_DOUBLE:
+       case BYTECODE_OP_UNARY_MINUS_DOUBLE:
+       case BYTECODE_OP_UNARY_NOT_DOUBLE:
+       {
+               next_pc += sizeof(struct unary_op);
+               break;
+       }
+
+       /* logical */
+       case BYTECODE_OP_AND:
+       case BYTECODE_OP_OR:
+       {
+               next_pc += sizeof(struct logical_op);
+               break;
+       }
+
+       /* load field ref */
+       case BYTECODE_OP_LOAD_FIELD_REF:
+       /* get context ref */
+       case BYTECODE_OP_GET_CONTEXT_REF:
+       {
+               next_pc += sizeof(struct load_op) + sizeof(struct field_ref);
+               break;
+       }
+       case BYTECODE_OP_LOAD_FIELD_REF_STRING:
+       case BYTECODE_OP_LOAD_FIELD_REF_SEQUENCE:
+       case BYTECODE_OP_GET_CONTEXT_REF_STRING:
+       case BYTECODE_OP_LOAD_FIELD_REF_S64:
+       case BYTECODE_OP_GET_CONTEXT_REF_S64:
+       case BYTECODE_OP_LOAD_FIELD_REF_DOUBLE:
+       case BYTECODE_OP_GET_CONTEXT_REF_DOUBLE:
+       {
+               /*
+                * Reject specialized load field ref instructions.
+                */
+               ret = -EINVAL;
+               goto end;
+       }
+
+       /* load from immediate operand */
+       case BYTECODE_OP_LOAD_STRING:
+       case BYTECODE_OP_LOAD_STAR_GLOB_STRING:
+       {
+               struct load_op *insn = (struct load_op *) pc;
+
+               next_pc += sizeof(struct load_op) + strlen(insn->data) + 1;
+               break;
+       }
+
+       case BYTECODE_OP_LOAD_S64:
+       {
+               next_pc += sizeof(struct load_op) + sizeof(struct literal_numeric);
+               break;
+       }
+       case BYTECODE_OP_LOAD_DOUBLE:
+       {
+               next_pc += sizeof(struct load_op) + sizeof(struct literal_double);
+               break;
+       }
+
+       case BYTECODE_OP_CAST_DOUBLE_TO_S64:
+       case BYTECODE_OP_CAST_TO_S64:
+       case BYTECODE_OP_CAST_NOP:
+       {
+               next_pc += sizeof(struct cast_op);
+               break;
+       }
+
+       /*
+        * Instructions for recursive traversal through composed types.
+        */
+       case BYTECODE_OP_GET_CONTEXT_ROOT:
+       case BYTECODE_OP_GET_APP_CONTEXT_ROOT:
+       case BYTECODE_OP_GET_PAYLOAD_ROOT:
+       case BYTECODE_OP_LOAD_FIELD:
+       {
+               next_pc += sizeof(struct load_op);
+               break;
+       }
+
+       case BYTECODE_OP_LOAD_FIELD_S8:
+       case BYTECODE_OP_LOAD_FIELD_S16:
+       case BYTECODE_OP_LOAD_FIELD_S32:
+       case BYTECODE_OP_LOAD_FIELD_S64:
+       case BYTECODE_OP_LOAD_FIELD_U8:
+       case BYTECODE_OP_LOAD_FIELD_U16:
+       case BYTECODE_OP_LOAD_FIELD_U32:
+       case BYTECODE_OP_LOAD_FIELD_U64:
+       case BYTECODE_OP_LOAD_FIELD_STRING:
+       case BYTECODE_OP_LOAD_FIELD_SEQUENCE:
+       case BYTECODE_OP_LOAD_FIELD_DOUBLE:
+       {
+               /*
+                * Reject specialized load field instructions.
+                */
+               ret = -EINVAL;
+               goto end;
+       }
+
+       case BYTECODE_OP_GET_SYMBOL:
+       case BYTECODE_OP_GET_SYMBOL_FIELD:
+       {
+               next_pc += sizeof(struct load_op) + sizeof(struct get_symbol);
+               break;
+       }
+
+       case BYTECODE_OP_GET_INDEX_U16:
+       {
+               next_pc += sizeof(struct load_op) + sizeof(struct get_index_u16);
+               break;
+       }
+
+       case BYTECODE_OP_GET_INDEX_U64:
+       {
+               next_pc += sizeof(struct load_op) + sizeof(struct get_index_u64);
+               break;
+       }
+
+       }
+end:
+       *_next_pc = next_pc;
+       return ret;
+}
+
 /*
  * Return value:
  * >0: going to next insn.
@@ -1907,6 +2130,32 @@ end:
        return ret;
 }
 
+int lttng_bytecode_validate_load(struct bytecode_runtime *bytecode)
+{
+       char *pc, *next_pc, *start_pc;
+       int ret = -EINVAL;
+
+       start_pc = &bytecode->code[0];
+       for (pc = next_pc = start_pc; pc - start_pc < bytecode->len;
+                       pc = next_pc) {
+               ret = bytecode_validate_overflow(bytecode, start_pc, pc);
+               if (ret != 0) {
+                       if (ret == -ERANGE)
+                               ERR("Bytecode overflow\n");
+                       goto end;
+               }
+               dbg_printf("Validating loads: op %s (%u)\n",
+                       lttng_bytecode_print_op((unsigned int) *(bytecode_opcode_t *) pc),
+                       (unsigned int) *(bytecode_opcode_t *) pc);
+
+               ret = validate_load(&next_pc, pc);
+               if (ret)
+                       goto end;
+       }
+end:
+       return ret;
+}
+
 /*
  * Never called concurrently (hash seed is shared).
  */

diff --git a/src/lib/lttng-ust/lttng-bytecode.c b/src/lib/lttng-ust/lttng-bytecode.c
index 099fc5ca5bab4d444f328fc2fa0ae9991cefe749..92258f168dd34beaeaf62b4fc7a7bdded27f582c 100644 (file)
--- a/src/lib/lttng-ust/lttng-bytecode.c
+++ b/src/lib/lttng-ust/lttng-bytecode.c
@@ -458,6 +458,11 @@ int link_bytecode(const struct lttng_ust_event_desc *event_desc,
        runtime->len = bytecode->bc.reloc_offset;
        /* copy original bytecode */
        memcpy(runtime->code, bytecode->bc.data, runtime->len);
+       /* Validate bytecode load instructions before relocs. */
+       ret = lttng_bytecode_validate_load(runtime);
+       if (ret) {
+               goto link_error;
+       }
        /*
         * apply relocs. Those are a uint16_t (offset in bytecode)
         * followed by a string (field name).

diff --git a/src/lib/lttng-ust/lttng-bytecode.h b/src/lib/lttng-ust/lttng-bytecode.h
index 87a90dd6bf6b3ff554f50e85a294f3a04f6a780a..16e79eb30fc65f35db94d54f2976f165f02a0442 100644 (file)
--- a/src/lib/lttng-ust/lttng-bytecode.h
+++ b/src/lib/lttng-ust/lttng-bytecode.h
@@ -324,6 +324,9 @@ void lttng_bytecode_sync_state(struct lttng_ust_bytecode_runtime *runtime)
 int lttng_bytecode_validate(struct bytecode_runtime *bytecode)
        __attribute__((visibility("hidden")));
 
+int lttng_bytecode_validate_load(struct bytecode_runtime *bytecode)
+       __attribute__((visibility("hidden")));
+
 int lttng_bytecode_specialize(const struct lttng_ust_event_desc *event_desc,
                struct bytecode_runtime *bytecode)
        __attribute__((visibility("hidden")));