[daap] Fix null pointer deref in parse_meta()

Description
NULL Pointer Dereference in daap_reply_playlists (src/httpd_daap.c:1483).

Analysis
When the meta parameter contains consecutive commas (e.g. "abc,,def"), The
function parse_meta does not increment nmeta correctly and may leave NULL
values in the meta array. After calling parse_meta, The function
daap_reply_playlists accesses the member dfm of meta[i], causing a NULL Pointer
Dereference.

Steps to reproduce
Sending the following request (followed by two CRLFs):

GET /databases/1/containers?meta=abc,,def HTTP/1.1

Thanks @archersec for finding and reporting. Fixes #1961.

Author: ejurgensen

src/dmap_common.c

@@ -43,9 +43,12 @@ dmap_get_fields_table(int *nfields)
 
 // This wrapper is so callers don't need to include dmap_fields_hash.h
 const struct dmap_field *
-dmap_find_field_wrapper(const char *str, int len)
+dmap_find_field_wrapper(const char *str)
 {
-  return dmap_find_field(str, len);
+  if (!str)
+    return NULL;
+
+  return dmap_find_field(str, strlen(str));
 }
 
 void

src/dmap_common.h

@@ -45,7 +45,7 @@ const struct dmap_field *
 dmap_get_fields_table(int *nfields);
 
 const struct dmap_field *
-dmap_find_field_wrapper(const char *str, int len);
+dmap_find_field_wrapper(const char *str);
 
 
 void

src/httpd_daap.c

@@ -592,64 +592,61 @@ query_params_set(struct query_params *qp, int *sort_headers, struct httpd_reques
   user_agent_filter(qp, hreq);
 }
 
+#define NMETA_MAX 128
 static int
 parse_meta(const struct dmap_field ***out_meta, const char *param)
 {
-  const struct dmap_field **meta;
-  char *ptr;
+  const struct dmap_field *meta[NMETA_MAX];
+  const struct dmap_field *dfield;
+  size_t out_size;
+  char *saveptr;
   char *field;
-  char *metastr;
+  char *param_copy;
+  bool is_duplicate;
   int nmeta;
   int i;
-  int n;
-
-  CHECK_NULL(L_DAAP, metastr = strdup(param));
-
-  nmeta = 1;
-  ptr = metastr;
-  while ((ptr = strchr(ptr + 1, ',')) && (strlen(ptr) > 1))
-    nmeta++;
-
-  DPRINTF(E_DBG, L_DAAP, "Asking for %d meta tags\n", nmeta);
-
-  CHECK_NULL(L_DAAP, meta = calloc(nmeta, sizeof(const struct dmap_field *)));
 
-  field = strtok_r(metastr, ",", &ptr);
-  for (i = 0; field != NULL && i < nmeta; i++)
+  CHECK_NULL(L_DAAP, param_copy = strdup(param));
+  field = strtok_r(param_copy, ",", &saveptr);
+  nmeta = 0;
+  for (; field && nmeta < NMETA_MAX; field = strtok_r(NULL, ",", &saveptr))
     {
-      for (n = 0; (n < i) && (strcmp(field, meta[n]->desc) != 0); n++);
-
-      if (n == i)
+      dfield = dmap_find_field_wrapper(field);
+      if (!dfield)
 	{
-	  meta[i] = dmap_find_field_wrapper(field, strlen(field));
-
-	  if (!meta[i])
-	    {
-	      DPRINTF(E_WARN, L_DAAP, "Could not find requested meta field '%s'\n", field);
-
-	      i--;
-	      nmeta--;
-	    }
+	  DPRINTF(E_DBG, L_DAAP, "Could not find requested meta field '%s' in '%s'\n", field, param);
+	  continue;
 	}
-      else
-	{
-	  DPRINTF(E_WARN, L_DAAP, "Parser will ignore duplicate occurrence of meta field '%s'\n", field);
 
-	  i--;
-	  nmeta--;
+      is_duplicate = false;
+      for (i = 0; i < nmeta && !is_duplicate; i++)
+	is_duplicate = (dfield == meta[i]);
+      if (is_duplicate)
+	{
+	  DPRINTF(E_SPAM, L_DAAP, "Parser will ignore duplicate occurrence of meta field '%s'\n", field);
+	  continue;
 	}
 
-      field = strtok_r(NULL, ",", &ptr);
+      meta[nmeta] = dfield;
+      nmeta++;
     }
 
-  free(metastr);
-
-  DPRINTF(E_DBG, L_DAAP, "Found %d meta tags\n", nmeta);
+  if (nmeta == 0)
+    {
+      printf("No known metadata fields found in input '%s'\n", param);
+      goto out;
+    }
 
-  *out_meta = meta;
+  // meta is a list of pointers to dmf entries, here we copy it to the heap
+  out_size = nmeta * sizeof(struct dmap_field *);
+  CHECK_NULL(L_DAAP, *out_meta = malloc(out_size));
+  memcpy(*out_meta, meta, out_size);
 
+ out:
+  free(param_copy);
   return nmeta;
 }
+#undef NMETA_MAX
 
 static void
 daap_reply_send(struct httpd_request *hreq, enum daap_reply_result result)