From 12e916466c99983f296903eebf7994105009da99 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Fri, 9 Sep 2016 14:52:18 -0400 Subject: [PATCH] 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 | 5 ++- src/libostree/ostree-core.c | 24 +++++++++-- src/libostree/ostree-repo-static-delta-core.c | 14 +++++-- src/libostree/ostree-repo.c | 4 +- tests/test-checksum.c | 42 +++++++++++++++---- tests/test-delta.sh | 9 +++- 6 files changed, 77 insertions(+), 21 deletions(-) diff --git a/src/libostree/ostree-core-private.h b/src/libostree/ostree-core-private.h index cb4d953d..0c5fb0eb 100644 --- a/src/libostree/ostree-core-private.h +++ b/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, diff --git a/src/libostree/ostree-core.c b/src/libostree/ostree-core.c index 6e3c4e2e..e3f0a771 100644 --- a/src/libostree/ostree-core.c +++ b/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 -_ostree_parse_delta_name (const char *delta_name, +gboolean +_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; } /* diff --git a/src/libostree/ostree-repo-static-delta-core.c b/src/libostree/ostree-repo-static-delta-core.c index 46fa5f86..82c80a2a 100644 --- a/src/libostree/ostree-repo-static-delta-core.c +++ b/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, diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index 59bfbf9e..e4e1ecbf 100644 --- a/src/libostree/ostree-repo.c +++ b/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) diff --git a/tests/test-checksum.c b/tests/test-checksum.c index 9537804d..7d2a0cea 100644 --- a/tests/test-checksum.c +++ b/tests/test-checksum.c @@ -31,25 +31,49 @@ static void test_ostree_parse_delta_name (void) { { - g_autofree char *from; - g_autofree char *to; - _ostree_parse_delta_name ("30d13b73cfe1e6988ffc345eac905f82a18def8ef1f0666fc392019e9eac388d", &from, &to); + g_autofree char *from = NULL; + g_autofree char *to = NULL; + 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 *to; - _ostree_parse_delta_name ("30d13b73cfe1e6988ffc345eac905f82a18def8ef1f0666fc392019e9eac388d-5891b5b522d5df086d0ff0b110fbd9d21bb4fc7163af34d08286a2e846f6be03", &from, &to); + g_autofree char *from = NULL; + g_autofree char *to = NULL; + 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 *to; - _ostree_parse_delta_name ("", &from, &to); + g_autofree char *from = NULL; + g_autofree char *to = NULL; + 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); } diff --git a/tests/test-delta.sh b/tests/test-delta.sh index f8209651..bb523b47 100755 --- a/tests/test-delta.sh +++ b/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'