diff --git a/system/nxpkg/pkg_manifest.c b/system/nxpkg/pkg_manifest.c index 3e7b56a2637..dca17546c21 100644 --- a/system/nxpkg/pkg_manifest.c +++ b/system/nxpkg/pkg_manifest.c @@ -78,6 +78,49 @@ static bool pkg_validate_hex(FAR const char *value) * Public Functions ****************************************************************************/ +/**************************************************************************** + * Name: pkg_validate_path_component + * + * Description: + * Reject any value that could escape the intended directory when spliced + * into a filesystem path (pkg_store.c's PKG_STORE_DIR "/%s/%s/..." + * formatters). This is required for "name" and "version" specifically, + * since both come straight from an untrusted, network-fetched + * index.json and are used unsanitized to build install paths - a + * version of "../../evil" would otherwise let a malicious index write + * or delete files outside the package store entirely. + * + ****************************************************************************/ + +bool pkg_validate_path_component(FAR const char *value) +{ + FAR const char *p; + + if (pkg_validate_required(value) < 0) + { + return false; + } + + /* Reject a leading '.' outright: blocks ".", "..", and any + * "../"-prefixed traversal in one check. + */ + + if (value[0] == '.') + { + return false; + } + + for (p = value; *p != '\0'; p++) + { + if (*p == '/' || *p == '\\') + { + return false; + } + } + + return true; +} + const char *pkg_manifest_type_str(enum pkg_payload_type_e type) { switch (type) @@ -95,6 +138,8 @@ const char *pkg_manifest_type_str(enum pkg_payload_type_e type) int pkg_manifest_validate(FAR const struct pkg_manifest_s *manifest) { + size_t i; + if (manifest == NULL) { return -EINVAL; @@ -110,6 +155,19 @@ int pkg_manifest_validate(FAR const struct pkg_manifest_s *manifest) return -EINVAL; } + /* "name" and "version" get spliced unsanitized into on-disk paths + * (pkg_store.c) - they must not contain path separators or traversal + * sequences. "artifact" is validated separately in pkg_repo.c, where + * it's legitimately allowed to be a relative repo path (just not an + * absolute one or one that escapes the repo root). + */ + + if (!pkg_validate_path_component(manifest->name) || + !pkg_validate_path_component(manifest->version)) + { + return -EINVAL; + } + if (strlen(manifest->sha256) != PKG_HASH_HEX_LEN) { return -EINVAL; @@ -126,6 +184,19 @@ int pkg_manifest_validate(FAR const struct pkg_manifest_s *manifest) return -EINVAL; } + if (manifest->launch_argc > PKG_LAUNCH_ARGS_MAX) + { + return -EINVAL; + } + + for (i = 0; i < manifest->launch_argc; i++) + { + if (pkg_validate_required(manifest->launch_args[i]) < 0) + { + return -EINVAL; + } + } + return 0; } diff --git a/system/nxpkg/pkg_metadata.c b/system/nxpkg/pkg_metadata.c index aadb4692e58..951b4772bf8 100644 --- a/system/nxpkg/pkg_metadata.c +++ b/system/nxpkg/pkg_metadata.c @@ -24,6 +24,7 @@ * Included Files ****************************************************************************/ +#include #include #include #include @@ -100,6 +101,54 @@ static FAR cJSON *pkg_metadata_packages_array(FAR cJSON *root) return cJSON_GetObjectItemCaseSensitive(root, "packages"); } +static int pkg_metadata_parse_launch_args( + FAR cJSON *item, FAR struct pkg_manifest_s *manifest) +{ + FAR cJSON *field; + FAR cJSON *arg; + size_t argc = 0; + FAR const char *value; + int ret; + + field = cJSON_GetObjectItemCaseSensitive(item, "launch_args"); + if (field == NULL) + { + manifest->launch_argc = 0; + return 0; + } + + if (!cJSON_IsArray(field)) + { + return -EINVAL; + } + + cJSON_ArrayForEach(arg, field) + { + if (argc >= PKG_LAUNCH_ARGS_MAX) + { + return -E2BIG; + } + + value = cJSON_GetStringValue(arg); + if (value == NULL) + { + return -EINVAL; + } + + ret = pkg_copy_string(manifest->launch_args[argc], + sizeof(manifest->launch_args[argc]), value); + if (ret < 0) + { + return ret; + } + + argc++; + } + + manifest->launch_argc = argc; + return 0; +} + static int pkg_metadata_parse_manifest(FAR cJSON *item, FAR struct pkg_manifest_s *manifest) { @@ -170,6 +219,39 @@ static int pkg_metadata_parse_manifest(FAR cJSON *item, return -EINVAL; } + /* description/category/icon are optional and purely for UI display; + * missing fields just leave the manifest's copy empty. + */ + + field = cJSON_GetObjectItemCaseSensitive(item, "description"); + value = cJSON_GetStringValue(field); + if (value != NULL) + { + pkg_copy_string(manifest->description, sizeof(manifest->description), + value); + } + + field = cJSON_GetObjectItemCaseSensitive(item, "category"); + value = cJSON_GetStringValue(field); + if (value != NULL) + { + pkg_copy_string(manifest->category, sizeof(manifest->category), + value); + } + + field = cJSON_GetObjectItemCaseSensitive(item, "icon"); + value = cJSON_GetStringValue(field); + if (value != NULL) + { + pkg_copy_string(manifest->icon, sizeof(manifest->icon), value); + } + + ret = pkg_metadata_parse_launch_args(item, manifest); + if (ret < 0) + { + return ret; + } + return pkg_manifest_validate(manifest); } @@ -221,6 +303,9 @@ static int pkg_metadata_parse_installed_entry( { FAR cJSON *field; FAR const char *value; + bool current_found = false; + bool previous_found = false; + size_t i; int ret; memset(entry, 0, sizeof(*entry)); @@ -285,38 +370,82 @@ static int pkg_metadata_parse_installed_entry( return ret; } + if (!pkg_validate_path_component(entry->name) || + !pkg_validate_path_component(entry->current) || + (entry->previous[0] != '\0' && + !pkg_validate_path_component(entry->previous))) + { + return -EINVAL; + } + + for (i = 0; i < entry->version_count; i++) + { + if (!pkg_validate_path_component(entry->versions[i])) + { + return -EINVAL; + } + + current_found |= strcmp(entry->versions[i], entry->current) == 0; + previous_found |= strcmp(entry->versions[i], entry->previous) == 0; + } + + if (!current_found || + (entry->previous[0] != '\0' && !previous_found)) + { + return -EINVAL; + } + return 0; } static int pkg_metadata_version_token_cmp(FAR const char *lhs, FAR const char *rhs) { - long leftnum; - long rightnum; - FAR char *leftend; - FAR char *rightend; + FAR const char *cmpleft; + FAR const char *cmpright; + FAR const char *leftdigits; + FAR const char *rightdigits; + size_t leftlen; + size_t rightlen; + int ret; - leftnum = strtol(lhs, &leftend, 10); - rightnum = strtol(rhs, &rightend, 10); + leftdigits = lhs; + rightdigits = rhs; + while (isdigit((unsigned char)*leftdigits)) + { + leftdigits++; + } + + while (isdigit((unsigned char)*rightdigits)) + { + rightdigits++; + } - if (leftend != lhs && rightend != rhs) + if (leftdigits != lhs && rightdigits != rhs) { - if (leftnum < rightnum) + while (*lhs == '0' && lhs + 1 < leftdigits) + { + lhs++; + } + + while (*rhs == '0' && rhs + 1 < rightdigits) + { + rhs++; + } + + leftlen = (size_t)(leftdigits - lhs); + rightlen = (size_t)(rightdigits - rhs); + if (leftlen < rightlen) { return -1; } - if (leftnum > rightnum) + if (leftlen > rightlen) { return 1; } - } - else - { - int ret; - ret = pkg_string_cmp(lhs, PKG_VERSION_MAX + 1, - rhs, PKG_VERSION_MAX + 1); + ret = memcmp(lhs, rhs, leftlen); if (ret < 0) { return -1; @@ -326,6 +455,26 @@ static int pkg_metadata_version_token_cmp(FAR const char *lhs, { return 1; } + + cmpleft = leftdigits; + cmpright = rightdigits; + } + else + { + cmpleft = lhs; + cmpright = rhs; + } + + ret = pkg_string_cmp(cmpleft, PKG_VERSION_MAX + 1, + cmpright, PKG_VERSION_MAX + 1); + if (ret < 0) + { + return -1; + } + + if (ret > 0) + { + return 1; } return 0; @@ -395,6 +544,8 @@ static FAR cJSON *pkg_metadata_manifest_to_json( FAR const struct pkg_manifest_s *manifest) { FAR cJSON *root; + FAR cJSON *launch_args; + size_t i; root = cJSON_CreateObject(); if (root == NULL) @@ -410,51 +561,55 @@ static FAR cJSON *pkg_metadata_manifest_to_json( cJSON_AddStringToObject(root, "sha256", manifest->sha256); cJSON_AddStringToObject(root, "type", pkg_manifest_type_str(manifest->type)); - return root; -} -/**************************************************************************** - * Public Functions - ****************************************************************************/ - -int pkg_metadata_load_index(FAR struct pkg_index_s *index) -{ - FAR cJSON *root; - FAR cJSON *packages; - FAR cJSON *item; - FAR char *text; - char path[PATH_MAX]; - size_t count = 0; - size_t textlen; - int ret; - - if (index == NULL) + if (manifest->description[0] != '\0') { - return -EINVAL; + cJSON_AddStringToObject(root, "description", manifest->description); } - memset(index, 0, sizeof(*index)); - - ret = pkg_store_format_index_path(path, sizeof(path)); - if (ret < 0) + if (manifest->category[0] != '\0') { - return ret; + cJSON_AddStringToObject(root, "category", manifest->category); } - pkg_info("loading index from %s", path); - - ret = pkg_store_read_text(path, &text); - if (ret < 0) + if (manifest->launch_argc > 0) { - return ret; + launch_args = cJSON_AddArrayToObject(root, "launch_args"); + if (launch_args == NULL) + { + cJSON_Delete(root); + return NULL; + } + + for (i = 0; i < manifest->launch_argc; i++) + { + FAR cJSON *arg; + + arg = cJSON_CreateString(manifest->launch_args[i]); + if (arg == NULL) + { + cJSON_Delete(root); + return NULL; + } + + cJSON_AddItemToArray(launch_args, arg); + } } - textlen = strlen(text); - pkg_info("index read complete (%zu bytes)", textlen); + return root; +} + +static int pkg_metadata_parse_index_text(FAR const char *text, + FAR struct pkg_index_s *index) +{ + FAR cJSON *root; + FAR cJSON *packages; + FAR cJSON *item; + size_t count = 0; + int ret; root = cJSON_Parse(text); pkg_info("cJSON_Parse returned %s", root != NULL ? "success" : "failure"); - free(text); if (root == NULL) { return -EINVAL; @@ -471,15 +626,27 @@ int pkg_metadata_load_index(FAR struct pkg_index_s *index) { if (count >= PKG_INDEX_MAX) { - cJSON_Delete(root); - return -E2BIG; + /* Keep what's already parsed rather than discarding the whole + * index: a catalog that's grown past PKG_INDEX_MAX shouldn't + * make every other package unavailable too. + */ + + pkg_error("index has more than %d packages, truncating", + PKG_INDEX_MAX); + break; } ret = pkg_metadata_parse_manifest(item, &index->manifests[count]); if (ret < 0) { - cJSON_Delete(root); - return ret; + /* Skip a malformed entry instead of discarding the entire + * index: one bad/malicious package definition shouldn't make + * every other, otherwise-valid package unavailable too. + */ + + pkg_error("skipping malformed package entry %zu: %d", count, + ret); + continue; } pkg_info("parsed manifest %s %s", @@ -493,6 +660,84 @@ int pkg_metadata_load_index(FAR struct pkg_index_s *index) return 0; } +/**************************************************************************** + * Public Functions + ****************************************************************************/ + +int pkg_metadata_load_index_path(FAR const char *path, + FAR struct pkg_index_s *index) +{ + FAR char *text; + size_t textlen; + int ret; + + if (path == NULL || index == NULL) + { + return -EINVAL; + } + + memset(index, 0, sizeof(*index)); + + pkg_info("loading index from %s", path); + + ret = pkg_store_read_text(path, &text); + if (ret < 0) + { + return ret; + } + + textlen = strlen(text); + pkg_info("index read complete (%zu bytes)", textlen); + + ret = pkg_metadata_parse_index_text(text, index); + pkg_free(text); + return ret; +} + +int pkg_metadata_load_index(FAR struct pkg_index_s *index) +{ + char path[PATH_MAX]; + int ret; + + ret = pkg_store_format_index_path(path, sizeof(path)); + if (ret < 0) + { + return ret; + } + + return pkg_metadata_load_index_path(path, index); +} + +int pkg_metadata_load_manifest_path(FAR const char *path, + FAR struct pkg_manifest_s *manifest) +{ + FAR cJSON *root; + FAR char *text; + int ret; + + if (path == NULL || manifest == NULL) + { + return -EINVAL; + } + + ret = pkg_store_read_text(path, &text); + if (ret < 0) + { + return ret; + } + + root = cJSON_Parse(text); + pkg_free(text); + if (root == NULL) + { + return -EINVAL; + } + + ret = pkg_metadata_parse_manifest(root, manifest); + cJSON_Delete(root); + return ret; +} + FAR const struct pkg_manifest_s * pkg_metadata_find_latest(FAR const struct pkg_index_s *index, FAR const char *name) @@ -573,7 +818,7 @@ int pkg_metadata_load_installed(FAR struct pkg_installed_db_s *db) } root = cJSON_Parse(text); - free(text); + pkg_free(text); if (root == NULL) { return -EINVAL; @@ -590,15 +835,23 @@ int pkg_metadata_load_installed(FAR struct pkg_installed_db_s *db) { if (count >= PKG_INSTALLED_MAX) { - cJSON_Delete(root); - return -E2BIG; + pkg_error("installed db has more than %d entries, truncating", + PKG_INSTALLED_MAX); + break; } ret = pkg_metadata_parse_installed_entry(item, &db->entries[count]); if (ret < 0) { - cJSON_Delete(root); - return ret; + /* A single corrupted entry (plausible after a crash mid-write, + * despite the atomic-write mechanism) must not make every + * other installed package look uninstalled - that would drive + * needless reinstalls for everything else. + */ + + pkg_error("skipping malformed installed entry %zu: %d", count, + ret); + continue; } count++;