Merge pull request #1502 from ctiller/check-on-headers

Validate that headers contain legal bytes

include/grpc/grpc.h

@@ -140,7 +140,9 @@ typedef enum grpc_call_error {
   /* there is already an outstanding read/write operation on the call */
   GRPC_CALL_ERROR_TOO_MANY_OPERATIONS,
   /* the flags value was illegal for this call */
-  GRPC_CALL_ERROR_INVALID_FLAGS
+  GRPC_CALL_ERROR_INVALID_FLAGS,
+  /* invalid metadata was passed to this call */
+  GRPC_CALL_ERROR_INVALID_METADATA
 } grpc_call_error;
 
 /* Result of a grpc operation */

src/core/surface/call.c

@@ -739,14 +739,9 @@ static void call_on_done_recv(void *pc, int success) {
   GRPC_TIMER_BEGIN(GRPC_PTAG_CALL_ON_DONE_RECV, 0);
 }
 
-static grpc_mdelem_list chain_metadata_from_app(grpc_call *call, size_t count,
+static int prepare_application_metadata(grpc_call *call, size_t count,
-                                                grpc_metadata *metadata) {
+                                        grpc_metadata *metadata) {
   size_t i;
-  grpc_mdelem_list out;
-  if (count == 0) {
-    out.head = out.tail = NULL;
-    return out;
-  }
   for (i = 0; i < count; i++) {
     grpc_metadata *md = &metadata[i];
     grpc_metadata *next_md = (i == count - 1) ? NULL : &metadata[i + 1];
@@ -756,9 +751,27 @@ static grpc_mdelem_list chain_metadata_from_app(grpc_call *call, size_t count,
     l->md = grpc_mdelem_from_string_and_buffer(call->metadata_context, md->key,
                                                (const gpr_uint8 *)md->value,
                                                md->value_length);
+    if (!grpc_mdstr_is_legal_header(l->md->key)) {
+      gpr_log(GPR_ERROR, "attempt to send invalid metadata key");
+      return 0;
+    } else if (!grpc_mdstr_is_bin_suffixed(l->md->key) &&
+               !grpc_mdstr_is_legal_header(l->md->value)) {
+      gpr_log(GPR_ERROR, "attempt to send invalid metadata value");
+      return 0;
+    }
     l->next = next_md ? (grpc_linked_mdelem *)&next_md->internal_data : NULL;
     l->prev = prev_md ? (grpc_linked_mdelem *)&prev_md->internal_data : NULL;
   }
+  return 1;
+}
+
+static grpc_mdelem_list chain_metadata_from_app(grpc_call *call, size_t count,
+                                                grpc_metadata *metadata) {
+  grpc_mdelem_list out;
+  if (count == 0) {
+    out.head = out.tail = NULL;
+    return out;
+  }
   out.head = (grpc_linked_mdelem *)&(metadata[0].internal_data);
   out.tail = (grpc_linked_mdelem *)&(metadata[count - 1].internal_data);
   return out;
@@ -954,8 +967,16 @@ static grpc_call_error start_ioreq(grpc_call *call, const grpc_ioreq *reqs,
     } else if (call->request_set[op] == REQSET_DONE) {
       return start_ioreq_error(call, have_ops, GRPC_CALL_ERROR_ALREADY_INVOKED);
     }
-    have_ops |= 1u << op;
     data = reqs[i].data;
+    if (op == GRPC_IOREQ_SEND_INITIAL_METADATA ||
+        op == GRPC_IOREQ_SEND_TRAILING_METADATA) {
+      if (!prepare_application_metadata(call, data.send_metadata.count,
+                                        data.send_metadata.metadata)) {
+        return start_ioreq_error(call, have_ops,
+                                 GRPC_CALL_ERROR_INVALID_METADATA);
+      }
+    }
+    have_ops |= 1u << op;
 
     call->request_data[op] = data;
     call->request_set[op] = set;

src/core/transport/metadata.c

@@ -569,3 +569,19 @@ void grpc_mdctx_locked_mdelem_unref(grpc_mdctx *ctx, grpc_mdelem *gmd) {
 }
 
 void grpc_mdctx_unlock(grpc_mdctx *ctx) { unlock(ctx); }
+
+int grpc_mdstr_is_legal_header(grpc_mdstr *s) {
+  /* TODO(ctiller): consider caching this, or computing it on construction */
+  const gpr_uint8 *p = GPR_SLICE_START_PTR(s->slice);
+  const gpr_uint8 *e = GPR_SLICE_END_PTR(s->slice);
+  for (; p != e; p++) {
+    if (*p < 32 || *p > 126) return 0;
+  }
+  return 1;
+}
+
+int grpc_mdstr_is_bin_suffixed(grpc_mdstr *s) {
+  /* TODO(ctiller): consider caching this */
+  return grpc_is_binary_header((const char *)GPR_SLICE_START_PTR(s->slice),
+                               GPR_SLICE_LENGTH(s->slice));
+}

src/core/transport/metadata.h

@@ -135,6 +135,9 @@ void grpc_mdelem_unref(grpc_mdelem *md);
    Does not promise that the returned string has no embedded nulls however. */
 const char *grpc_mdstr_as_c_string(grpc_mdstr *s);
 
+int grpc_mdstr_is_legal_header(grpc_mdstr *s);
+int grpc_mdstr_is_bin_suffixed(grpc_mdstr *s);
+
 /* Batch mode metadata functions.
    These API's have equivalents above, but allow taking the mdctx just once,
    performing a bunch of work, and then leaving the mdctx. */