CVE-2021-43114 (C, published 2021-11-09T13:15Z)
Description: value: FORT Validator versions prior to 1.5.2 will crash if an RPKI CA publishes an X.509 EE certificate. This will lead to RTR clients such as BGP routers to lose access to the RPKI VRP data set, effectively disabling Route Origin Validation.
Severity: MEDIUM (CVSS v3 base score: 7.5)
Weakness: NVD-CWE-Other \u2014 Other
NVD is only using a subset of CWE for mapping instead of the entire CWE, and the weakness type is not covered by that subset.
Repository: https://github.com/NICMx/FORT-validator
Files changed:
src/cert_stack.c
src/object/certificate.c
Commit message: Certificates: Fuse meta and level stacks

These stacks always had the same size, and their corresponding elements
always referred to the same certificate.

This was pending work from #55, which I think is now properly solved.

Also refactors x509stack_push(); was messy. Patched an unlikely memory
leak in the chaos.


Vulnerable code:
/**
 * Certificate repository "level". This aims to identify if the
 * certificate is located at a distinct server than its father (common
 * case when the RIRs delegate RPKI repositories).
 */
struct repo_level_node {
	unsigned int level;
	SLIST_ENTRY(repo_level_node) next;
};

SLIST_HEAD(repo_level_stack, repo_level_node);


	/**
	 * Stacked data to store the repository "levels" (each level is a
	 * delegation of an RPKI server).
	 */
	struct repo_level_stack levels;
	SLIST_INIT(&stack->levels);
	struct repo_level_node *level;
	stack_size = 0;
	while (!SLIST_EMPTY(&stack->levels)) {
		level = SLIST_FIRST(&stack->levels);
		SLIST_REMOVE_HEAD(&stack->levels, next);
		free(level);
		stack_size++;
	}
	pr_val_debug("Deleted %u stacked levels.", stack_size);

	struct repo_level_node *repo;

	repo = SLIST_FIRST(&stack->levels);
	if (repo == NULL)
		pr_crit("Attempted to pop empty repo level stack");
	SLIST_REMOVE_HEAD(&stack->levels, next);
	free(repo);
	struct repo_level_node *repo, *head_repo;
	unsigned int work_repo_level;
	repo = malloc(sizeof(struct repo_level_node));
	if (repo == NULL)
		return pr_enomem();

	repo->level = 0;
	work_repo_level = working_repo_peek_level();
	head_repo = SLIST_FIRST(&stack->levels);
	if (head_repo != NULL && work_repo_level > head_repo->level)
		repo->level = work_repo_level;

	SLIST_INSERT_HEAD(&stack->levels, repo, next);

	if (meta == NULL) {
		error = pr_enomem();
		goto end3;
	}
	meta->resources = resources_create(false);
	if (meta->resources == NULL) {
		error = pr_enomem();
		goto end4;
	}
	resources_set_policy(meta->resources, policy);
	error = certificate_get_resources(x509, meta->resources, type);
		goto end5;
	/*
	 * rfc8630#section-2.3
	 * "The INR extension(s) of this TA MUST contain a non-empty set of
	 * number resources."
	 * The "It MUST NOT use the "inherit" form of the INR extension(s)"
	 * part is already handled in certificate_get_resources().
	 */
	if (type == TA && resources_empty(meta->resources)) {
		error = pr_val_err("Trust Anchor certificate does not define any number resources.");
		goto end5;
	}
	defer_separator = malloc(sizeof(struct defer_node));
		goto end5;
	defer_separator->type = DNT_SEPARATOR;
		goto end5;
end5:	resources_destroy(meta->resources);
end4:	subjects_cleanup(&meta->subjects, subject_cleanup);
end3:	SLIST_REMOVE_HEAD(&stack->levels, next);
	free(repo);
	struct repo_level_node *repo = SLIST_FIRST(&stack->levels);
	return (repo != NULL) ? repo->level : 0;

Fixed code:
	/*
	 * Certificate repository "level". This aims to identify if the
	 * certificate is located at a distinct server than its father (common
	 * case when the RIRs delegate RPKI repositories).
	 */
	unsigned int level;

static int
init_resources(X509 *x509, enum rpki_policy policy, enum cert_type type,
    struct resources **_result)
{
	struct resources *result;
	int error;

	result = resources_create(false);
	if (result == NULL)
		return pr_enomem();

	resources_set_policy(result, policy);
	error = certificate_get_resources(x509, result, type);
	if (error)
		goto fail;

	/*
	 * rfc8630#section-2.3
	 * "The INR extension(s) of this TA MUST contain a non-empty set of
	 * number resources."
	 * The "It MUST NOT use the "inherit" form of the INR extension(s)"
	 * part is already handled in certificate_get_resources().
	 */
	if (type == TA && resources_empty(result)) {
		error = pr_val_err("Trust Anchor certificate does not define any number resources.");
		goto fail;
	}

	*_result = result;
	return 0;

fail:
	resources_destroy(result);
	return error;
}

static int
init_level(struct cert_stack *stack, unsigned int *_result)
{
	struct metadata_node *head_meta;
	unsigned int work_repo_level;
	unsigned int result;

	/*
	 * Bruh, I don't understand the point of this block.
	 * Why can't it just be `result = working_repo_peek_level();`?
	 */

	result = 0;
	work_repo_level = working_repo_peek_level();
	head_meta = SLIST_FIRST(&stack->metas);
	if (head_meta != NULL && work_repo_level > head_meta->level)
		result = work_repo_level;

	*_result = result;
	return 0;
}

static struct defer_node *
create_separator(void)
{
	struct defer_node *result;

	result = malloc(sizeof(struct defer_node));
	if (result == NULL)
		return NULL;

	result->type = DNT_SEPARATOR;
	return result;
}

	if (meta == NULL)
		return pr_enomem();
	error = init_resources(x509, policy, type, &meta->resources);
		goto cleanup_subjects;
	error = init_level(stack, &meta->level); /* Does not need a revert */
	if (error)
		goto destroy_resources;
	defer_separator = create_separator();
		goto destroy_resources;
		goto destroy_separator;
destroy_separator:
	free(defer_separator);
destroy_resources:
	resources_destroy(meta->resources);
cleanup_subjects:
	subjects_cleanup(&meta->subjects, subject_cleanup);
	struct metadata_node *meta = SLIST_FIRST(&stack->metas);
	return (meta != NULL) ? meta->level : 0;
/**
 * Copies the resources from @cert to @resources.
 */