Fix dependency expansion error handling

rust/flintlang/c-src/src/pkg.c

@@ -82,7 +82,8 @@ static PkgDep *pkg_expand_deps(PkgDep *list,
         int depth,
         char *error,
         size_t error_size,
-        int from);
+        int from,
+        bool *success);
 
 /*
  * Run git and capture its stdout. argv[0] is "git", the rest are plain
@@ -2093,6 +2094,7 @@ static int pkg_install_core(bool update_all, const char *update_one)
 	int count = 0;
 	if (!pkg_collect_deps(&manifest, &deps, &count, error, sizeof(error))) {
 		fprintf(stderr, "flint pkg: %s\n", error);
+		pkg_free_deps(deps, count);
 		pkg_free_manifest(&manifest);
 		return 65;
 	}
@@ -2152,6 +2154,7 @@ static int pkg_install_core(bool update_all, const char *update_one)
 		 * realloc, and reading the old pointer afterwards is the
 		 * use-after-free that produced a garbage path here. */
 		int filled = count;
+		bool expand_ok = true;
 		PkgDep *expanded = pkg_expand_deps(deps,
 		        count,
 		        &capacity,
@@ -2160,15 +2163,16 @@ static int pkg_install_core(bool update_all, const char *update_one)
 		        0,
 		        error,
 		        sizeof(error),
-		        0);
-		if (expanded == NULL) {
+		        0,
+		        &expand_ok);
+		deps = expanded;
+		count = filled;
+		if (!expand_ok) {
 			fprintf(stderr, "flint pkg: %s\n", error);
 			pkg_free_deps(deps, count);
 			pkg_free_locked(locked, locked_count);
 			return 65;
 		}
-		deps = expanded;
-		count = filled;
 
 		/* dedupe by name: two packages can reach the same one by
 		 * different routes, and it should be installed once. A name
@@ -2926,14 +2930,16 @@ static PkgDep *pkg_expand_deps(PkgDep *list,
         int depth,
         char *error,
         size_t error_size,
-        int from)
+        int from,
+        bool *success)
 {
-	if (from == 0 && depth > 32) {
+	if (depth > 32) {
 		snprintf(error,
 		        error_size,
 		        "dependency chain is deeper than 32 -- is there a "
 		        "cycle?");
-		return NULL;
+		*success = false;
+		return list;
 	}
 
 	/* count first: every append can move the array, so the loop below
@@ -2951,27 +2957,36 @@ static PkgDep *pkg_expand_deps(PkgDep *list,
 		 * reads as though it does not, and the lockfile should not
 		 * carry that spelling either.
 		 */
+		int path_len;
 		if (strcmp(base_dir, ".") == 0)
-			snprintf(manifest_path,
+			path_len = snprintf(manifest_path,
 			        sizeof(manifest_path),
 			        "%s/flint.toml",
 			        list[i].path);
 		else
-			snprintf(manifest_path,
+			path_len = snprintf(manifest_path,
 			        sizeof(manifest_path),
 			        "%s/%s/flint.toml",
 			        base_dir,
 			        list[i].path);
-		if (!pkg_load(manifest_path, &child, error, error_size))
-			return NULL;
+		if (path_len < 0 || (size_t)path_len >= sizeof(manifest_path)) {
+			snprintf(error, error_size, "dependency manifest path too long");
+			*success = false;
+			return list;
+		}
+		if (!pkg_load(manifest_path, &child, error, error_size)) {
+			*success = false;
+			return list;
+		}
 		PkgDep *nested = NULL;
 		int nested_count = 0;
 		bool ok = pkg_collect_deps(
 		        &child, &nested, &nested_count, error, error_size);
 		pkg_free_manifest(&child);
 		if (!ok) {
 			pkg_free_deps(nested, nested_count);
-			return NULL;
+			*success = false;
+			return list;
 		}
 		added += nested_count;
 		pkg_free_deps(nested, nested_count);
@@ -2990,7 +3005,8 @@ static PkgDep *pkg_expand_deps(PkgDep *list,
 		PkgDep *bigger = realloc(list, (size_t)grown * sizeof(PkgDep));
 		if (bigger == NULL) {
 			snprintf(error, error_size, "out of memory");
-			return NULL;
+			*success = false;
+			return list;
 		}
 		list = bigger;
 		*capacity = grown;
@@ -3016,27 +3032,36 @@ static PkgDep *pkg_expand_deps(PkgDep *list,
 		 * reads as though it does not, and the lockfile should not
 		 * carry that spelling either.
 		 */
+		int path_len;
 		if (strcmp(base_dir, ".") == 0)
-			snprintf(manifest_path,
+			path_len = snprintf(manifest_path,
 			        sizeof(manifest_path),
 			        "%s/flint.toml",
 			        list[i].path);
 		else
-			snprintf(manifest_path,
+			path_len = snprintf(manifest_path,
 			        sizeof(manifest_path),
 			        "%s/%s/flint.toml",
 			        base_dir,
 			        list[i].path);
-		if (!pkg_load(manifest_path, &child, error, error_size))
-			return NULL;
+		if (path_len < 0 || (size_t)path_len >= sizeof(manifest_path)) {
+			snprintf(error, error_size, "dependency manifest path too long");
+			*success = false;
+			return list;
+		}
+		if (!pkg_load(manifest_path, &child, error, error_size)) {
+			*success = false;
+			return list;
+		}
 		PkgDep *nested = NULL;
 		int nested_count = 0;
 		bool ok = pkg_collect_deps(
 		        &child, &nested, &nested_count, error, error_size);
 		pkg_free_manifest(&child);
 		if (!ok) {
 			pkg_free_deps(nested, nested_count);
-			return NULL;
+			*success = false;
+			return list;
 		}
 		for (int k = 0; k < nested_count; k++) {
 			/* a nested path is relative to its parent package */
@@ -3048,24 +3073,46 @@ static PkgDep *pkg_expand_deps(PkgDep *list,
 				 * it, and "." must not contribute a
 				 * component or the lockfile carries a
 				 * spelling nobody wrote. */
+				int joined_len;
 				if (strcmp(base_dir, ".") == 0)
-					snprintf(joined,
+					joined_len = snprintf(joined,
 					        sizeof(joined),
 					        "%s",
 					        list[i].path);
 				else
-					snprintf(joined,
+					joined_len = snprintf(joined,
 					        sizeof(joined),
 					        "%s/%s",
 					        base_dir,
 					        list[i].path);
+				if (joined_len < 0 ||
+				        (size_t)joined_len >= sizeof(joined)) {
+					pkg_free_deps(nested, nested_count);
+					free(nested);
+					snprintf(error,
+					        error_size,
+					        "dependency path is too long");
+					*success = false;
+					return list;
+				}
 				char *absolute = pkg_normalize_path(
 				        joined, nested[k].path);
 				free(nested[k].path);
+				if (absolute == NULL) {
+					nested[k].path = NULL;
+					pkg_free_deps(nested, nested_count);
+					free(nested);
+					snprintf(error, error_size, "out of memory");
+					*success = false;
+					return list;
+				}
 				nested[k].path = absolute;
 			}
 			list[count++] = nested[k];
+			memset(&nested[k], 0, sizeof(nested[k]));
+			*filled = count;
 		}
+		free(nested);
 	}
 
 	*filled = count;
@@ -3078,5 +3125,6 @@ static PkgDep *pkg_expand_deps(PkgDep *list,
 	        depth + 1,
 	        error,
 	        error_size,
-	        start);
+	        start,
+	        success);
 }