static-delta: add some error handling

We make _ostree_parse_delta_name() a bit more defensive since it handles
user input.

Closes: #504

Closes: #505
Approved by: cgwalters

src/libostree/ostree-core-private.h

@@ -121,10 +121,11 @@ static inline char * _ostree_get_commitpartial_path (const char *checksum)
   return g_strconcat ("state/", checksum, ".commitpartial", NULL);
 }
 
-void
+gboolean
 _ostree_parse_delta_name (const char  *delta_name,
                           char        **out_from,
-                          char        **out_to);
+                          char        **out_to,
+                          GError      **error);
 
 void
 _ostree_loose_path (char              *buf,

src/libostree/ostree-core.c

@@ -1584,12 +1584,26 @@ _ostree_get_relative_static_delta_part_path (const char        *from,
   return _ostree_get_relative_static_delta_path (from, to, partstr);
 }
 
-void
+gboolean
-_ostree_parse_delta_name (const char  *delta_name,
+_ostree_parse_delta_name (const char   *delta_name,
                           char        **out_from,
-                          char        **out_to)
+                          char        **out_to,
+                          GError      **error)
 {
-  g_auto(GStrv) parts = g_strsplit (delta_name, "-", 2);
+  g_auto(GStrv) parts = NULL;
+  g_return_val_if_fail (delta_name != NULL, FALSE);
+
+  parts = g_strsplit (delta_name, "-", 2);
+
+  /* NB: if delta_name is "", parts[0] is NULL, but the error
+   * validate_checksum_string() gives for "" is nice enough,
+   * so we just coerce it here */
+  if (!ostree_validate_checksum_string (parts[0] ?: "", error))
+    return FALSE;
+
+  if (parts[0] && parts[1] &&
+      !ostree_validate_checksum_string (parts[1], error))
+    return FALSE;
 
   *out_from = *out_to = NULL;
   if (parts[0] && parts[1])
@@ -1601,6 +1615,8 @@ _ostree_parse_delta_name (const char  *delta_name,
     {
       ot_transfer_out_value (out_to, &parts[0]);
     }
+
+  return TRUE;
 }
 
 /*

src/libostree/ostree-repo-static-delta-core.c

@@ -795,7 +795,9 @@ _ostree_repo_static_delta_delete (OstreeRepo                    *self,
   g_autofree char *deltadir = NULL;
   struct stat buf;
 
-  _ostree_parse_delta_name (delta_id, &from, &to);
+  if (!_ostree_parse_delta_name (delta_id, &from, &to, error))
+    goto out;
+
   deltadir = _ostree_get_relative_static_delta_path (from, to, NULL);
 
   if (fstatat (self->repo_dir_fd, deltadir, &buf, 0) != 0)
@@ -830,7 +832,9 @@ _ostree_repo_static_delta_query_exists (OstreeRepo                    *self,
   g_autofree char *superblock_path = NULL;
   struct stat stbuf;
 
-  _ostree_parse_delta_name (delta_id, &from, &to);
+  if (!_ostree_parse_delta_name (delta_id, &from, &to, error))
+    return FALSE;
+
   superblock_path = _ostree_get_relative_static_delta_superblock_path (from, to);
 
   if (fstatat (self->repo_dir_fd, superblock_path, &stbuf, 0) < 0)
@@ -854,7 +858,7 @@ gboolean
 _ostree_repo_static_delta_dump (OstreeRepo                    *self,
                                 const char                    *delta_id,
                                 GCancellable                  *cancellable,
-                                GError                      **error)
+                                GError                       **error)
 {
   gboolean ret = FALSE;
   g_autofree char *from = NULL; 
@@ -868,7 +872,9 @@ _ostree_repo_static_delta_dump (OstreeRepo                    *self,
   OstreeDeltaEndianness endianness;
   gboolean swap_endian = FALSE;
 
-  _ostree_parse_delta_name (delta_id, &from, &to);
+  if (!_ostree_parse_delta_name (delta_id, &from, &to, error))
+    goto out;
+
   superblock_path = _ostree_get_relative_static_delta_superblock_path (from, to);
 
   if (!ot_util_variant_map_at (self->repo_dir_fd, superblock_path,

src/libostree/ostree-repo.c

@@ -4701,7 +4701,9 @@ ostree_repo_regenerate_summary (OstreeRepo     *self,
         glnx_fd_close int superblock_file_fd = -1;
         g_autoptr(GInputStream) in_stream = NULL;
 
-        _ostree_parse_delta_name (delta_names->pdata[i], &from, &to);
+        if (!_ostree_parse_delta_name (delta_names->pdata[i], &from, &to, error))
+          goto out;
+
         superblock = _ostree_get_relative_static_delta_superblock_path ((from && from[0]) ? from : NULL, to);
         superblock_file_fd = openat (self->repo_dir_fd, superblock, O_RDONLY | O_CLOEXEC);
         if (superblock_file_fd == -1)

tests/test-checksum.c

@@ -31,25 +31,49 @@ static void
 test_ostree_parse_delta_name (void)
 {
   {
-    g_autofree char *from;
+    g_autofree char *from = NULL;
-    g_autofree char *to;
+    g_autofree char *to = NULL;
-    _ostree_parse_delta_name ("30d13b73cfe1e6988ffc345eac905f82a18def8ef1f0666fc392019e9eac388d", &from, &to);
+    g_assert (_ostree_parse_delta_name ("30d13b73cfe1e6988ffc345eac905f82a18def8ef1f0666fc392019e9eac388d", &from, &to, NULL));
     g_assert_cmpstr (to, ==, "30d13b73cfe1e6988ffc345eac905f82a18def8ef1f0666fc392019e9eac388d");
     g_assert_null (from);
   }
 
   {
-    g_autofree char *from;
+    g_autofree char *from = NULL;
-    g_autofree char *to;
+    g_autofree char *to = NULL;
-    _ostree_parse_delta_name ("30d13b73cfe1e6988ffc345eac905f82a18def8ef1f0666fc392019e9eac388d-5891b5b522d5df086d0ff0b110fbd9d21bb4fc7163af34d08286a2e846f6be03", &from, &to);
+    g_assert (_ostree_parse_delta_name ("30d13b73cfe1e6988ffc345eac905f82a18def8ef1f0666fc392019e9eac388d-5891b5b522d5df086d0ff0b110fbd9d21bb4fc7163af34d08286a2e846f6be03", &from, &to, NULL));
     g_assert_cmpstr (from, ==, "30d13b73cfe1e6988ffc345eac905f82a18def8ef1f0666fc392019e9eac388d");
     g_assert_cmpstr (to, ==, "5891b5b522d5df086d0ff0b110fbd9d21bb4fc7163af34d08286a2e846f6be03");
   }
 
   {
-    g_autofree char *from;
+    g_autofree char *from = NULL;
-    g_autofree char *to;
+    g_autofree char *to = NULL;
-    _ostree_parse_delta_name ("", &from, &to);
+    g_assert (!_ostree_parse_delta_name ("", &from, &to, NULL));
+    g_assert_null (from);
+    g_assert_null (to);
+  }
+
+  {
+    g_autofree char *from = NULL;
+    g_autofree char *to = NULL;
+    g_assert (!_ostree_parse_delta_name ("GARBAGE", &from, &to, NULL));
+    g_assert_null (from);
+    g_assert_null (to);
+  }
+
+  {
+    g_autofree char *from = NULL;
+    g_autofree char *to = NULL;
+    g_assert (!_ostree_parse_delta_name ("GARBAGE-5891b5b522d5df086d0ff0b110fbd9d21bb4fc7163af34d08286a2e846f6be03", &from, &to, NULL));
+    g_assert_null (from);
+    g_assert_null (to);
+  }
+
+  {
+    g_autofree char *from = NULL;
+    g_autofree char *to = NULL;
+    g_assert (!_ostree_parse_delta_name ("30d13b73cfe1e6988ffc345eac905f82a18def8ef1f0666fc392019e9eac388d-GARBAGE", &from, &to, NULL));
     g_assert_null (from);
     g_assert_null (to);
   }

tests/test-delta.sh

@@ -26,7 +26,7 @@ skip_without_user_xattrs
 bindatafiles="bash true ostree"
 morebindatafiles="false ls"
 
-echo '1..10'
+echo '1..11'
 
 mkdir repo
 ${CMD_PREFIX} ostree --repo=repo init --mode=archive-z2
@@ -235,3 +235,10 @@ ${CMD_PREFIX} ostree --repo=repo2 fsck
 ${CMD_PREFIX} ostree --repo=repo2 ls ${samerev} >/dev/null
 
 echo 'ok pull empty delta part'
+
+if ${CMD_PREFIX} ostree --repo=repo static-delta show GARBAGE 2> err.txt; then
+    assert_not_reached "static-delta show GARBAGE unexpectedly succeeded"
+fi
+assert_file_has_content err.txt "Invalid rev 'GARBAGE'"
+
+echo 'ok handle bad delta name'