* After the initial sort, the merge runs as a single linear sweep
* over the list using a write index. Adjacent entries are folded
* into the previous output by replacing it with a freshly built
- * merged range; both old entries are then freed and the source slot
- * is left NULL so the asn1 free machinery does not double-free on a
- * subsequent abort. Total cost is O(N log N) sort + O(N) merge,
- * with no stack deletes inside the loop.
+ * merged range; both old entries are then freed and the consumed
+ * source slot is recorded as NULL. Total cost is O(N log N) sort +
+ * O(N) merge, with no stack deletes inside the loop.
+ *
+ * The NULL slots are closed up by a single compaction pass at the end,
+ * which runs whether the sweep succeeded or failed. This guarantees
+ * the live stack is always left hole-free, with every occupied slot
+ * non-NULL, so on error the caller may safely inspect, print, encode,
+ * free, or retry canonize on the object. The sort comparator
+ * dereferences every slot with no NULL guard, so a retry (which
+ * re-sorts) would crash on a hole. is_canonical tolerates NULL by
+ * returning non-canonical, but the object is still structurally
+ * invalid.
*/
static int IPAddressOrRanges_canonize(IPAddressOrRanges *aors,
const unsigned afi)
{
int length = length_from_afi(afi);
int read, write = 0, n;
+ int ret = 0;
sk_IPAddressOrRange_sort(aors);
n = sk_IPAddressOrRange_num(aors);
- /*
- * Error paths below all `return 0` directly. Slots at
- * [write..read-1] are NULL (from earlier iterations) and slots at
- * [read..n-1] still hold their original entries; the caller's
- * normal teardown walks the whole stack and frees each non-NULL
- * slot safely, so leaving the stack in this mixed state is sound.
- */
for (read = 0; read < n; read++) {
IPAddressOrRange *cur = sk_IPAddressOrRange_value(aors, read);
unsigned char c_min[ADDR_RAW_BUF_LEN], c_max[ADDR_RAW_BUF_LEN];
if (!extract_min_max(cur, c_min, c_max, length))
- return 0;
+ goto done;
/*
* Punt inverted range.
*/
if (memcmp(c_min, c_max, length) > 0)
- return 0;
+ goto done;
if (write > 0) {
IPAddressOrRange *prev = sk_IPAddressOrRange_value(aors,
int j;
if (!extract_min_max(prev, p_min, p_max, length))
- return 0;
+ goto done;
/*
* Reject overlap with the previous accepted entry.
*/
if (memcmp(p_max, c_min, length) >= 0)
- return 0;
+ goto done;
/*
* Adjacency test: does c_min - 1 equal p_max? Work on a
IPAddressOrRange *merged;
if (!make_addressRange(&merged, p_min, c_max, length))
- return 0;
+ goto done;
/*
* Replace prev with merged, free the originals, and
- * NULL the source slot so the stack does not retain a
- * second reference to cur.
+ * record the consumed source slot as NULL; the epilogue
+ * compacts NULLs out of the live stack so it is left
+ * hole-free whether we succeed or fail.
*/
(void)sk_IPAddressOrRange_set(aors, write - 1, merged);
IPAddressOrRange_free(prev);
write++;
}
+ ret = 1;
+
+done:
/*
- * Compaction succeeded: every slot at [write..n-1] is NULL, so
- * popping the tail leaves the canonicalised list at [0..write-1].
+ * The sweep above NULLs source slots as it folds entries. Whether
+ * we succeeded or bailed out on an error, the stack must be left
+ * hole-free, with every occupied slot non-NULL, so that the caller
+ * may inspect, print, encode, free, or even retry canonize on the
+ * object without dereferencing NULL. Slide every non-NULL slot
+ * forward to close the holes, then pop the vacated tail. On success
+ * this collapses [write..n-1] (all NULL) to length `write`; on error
+ * it removes the possibly empty contiguous run of NULL slots in
+ * [write, read), preserving the processed output in [0, write) and
+ * the untouched original entries in [read, n).
*/
- while (sk_IPAddressOrRange_num(aors) > write)
- (void)sk_IPAddressOrRange_pop(aors);
- return 1;
+ {
+ int w = 0, r;
+
+ for (r = 0; r < sk_IPAddressOrRange_num(aors); r++) {
+ IPAddressOrRange *v = sk_IPAddressOrRange_value(aors, r);
+
+ if (v != NULL) {
+ if (w != r)
+ (void)sk_IPAddressOrRange_set(aors, w, v);
+ w++;
+ }
+ }
+ while (sk_IPAddressOrRange_num(aors) > w)
+ (void)sk_IPAddressOrRange_pop(aors);
+ }
+
+ return ret;
}
/*
* After the initial sort, the merge runs as a single linear sweep
* over the list using a write index. Each entry is examined once;
* adjacent / mergeable entries extend the previous output's upper
- * bound in O(1) and the source slot is left NULL so the asn1 free
- * machinery does not double-free on a subsequent abort. Total cost
- * is O(N log N) sort + O(N) merge, with no stack deletes inside the
- * loop.
+ * bound in O(1) and the consumed source slot is recorded as NULL.
+ * Total cost is O(N log N) sort + O(N) merge, with no stack deletes
+ * inside the loop.
+ *
+ * The NULL slots are closed up by a single compaction pass at the end,
+ * which runs whether the sweep succeeded or failed. This guarantees
+ * the live stack is always left hole-free, with every occupied slot
+ * non-NULL, so on error the caller may safely inspect, print, encode,
+ * free, or retry canonize on the object. The sort comparator
+ * dereferences every slot with no NULL guard, and is_canonical's call
+ * to extract_min_max would hit its ossl_assert(aor != NULL) on a hole.
*/
static int ASIdentifierChoice_canonize(ASIdentifierChoice *choice)
{
}
ASIdOrRange_free(cur);
/*
- * NULL the source slot so any later teardown does not
- * walk a freed pointer. We do not advance `write`.
+ * Record the consumed source slot as NULL; the epilogue
+ * compacts NULLs out of the live stack so it is left
+ * hole-free whether we succeed or fail. We do not
+ * advance `write`.
*/
(void)sk_ASIdOrRange_set(choice->u.asIdsOrRanges, read, NULL);
continue;
done:
/*
- * On success every slot at [write..n-1] is NULL, so popping the
- * tail leaves the canonicalised list at [0..write-1]. On error we
- * leave the tail untouched; the slots are either NULL (from earlier
- * iterations) or original entries the loop never reached, both of
- * which the caller's ASIdentifierChoice_free path handles safely.
+ * The sweep above NULLs source slots as it folds entries. Whether
+ * we succeeded or bailed out on an error, the stack must be left
+ * hole-free, with every occupied slot non-NULL, so that the caller
+ * may inspect, print, encode, free, or even retry canonize on the
+ * object without dereferencing NULL (the sort comparator in
+ * particular has no NULL guard). Slide every non-NULL slot forward
+ * to close the holes, then pop the vacated tail. On success this
+ * collapses [write..n-1] (all NULL) to length `write`; on error it
+ * removes the possibly empty contiguous run of NULL slots in
+ * [write, read), preserving the processed output in [0, write) and
+ * the untouched original entries in [read, n).
*/
- if (ret) {
- while (sk_ASIdOrRange_num(choice->u.asIdsOrRanges) > write)
+ {
+ int w = 0, r;
+
+ for (r = 0; r < sk_ASIdOrRange_num(choice->u.asIdsOrRanges); r++) {
+ ASIdOrRange *v = sk_ASIdOrRange_value(choice->u.asIdsOrRanges, r);
+
+ if (v != NULL) {
+ if (w != r)
+ (void)sk_ASIdOrRange_set(choice->u.asIdsOrRanges, w, v);
+ w++;
+ }
+ }
+ while (sk_ASIdOrRange_num(choice->u.asIdsOrRanges) > w)
(void)sk_ASIdOrRange_pop(choice->u.asIdsOrRanges);
+ }
+
+ if (ret) {
/* Paranoia */
if (!ossl_assert(ASIdentifierChoice_is_canonical(choice)))
ret = 0;
* Trigger an overlap-detection error partway through the linear
* merge. The first V3EXT_TEST_LARGE_N / 2 entries are adjacent and
* mergeable; entry K is a duplicate of entry K-1 (overlap). The
- * canonize call must return 0, and the caller's normal teardown of
- * the choice must safely free the stack -- some slots hold merged
- * results, some hold NULL (from earlier merges), and some hold
- * originals that the loop never reached. ASan / UBSan-instrumented
- * builds will catch any double-free or use-after-free in the
- * teardown that the mixed-state-on-error invariant claims to avoid.
+ * canonize call must return 0, and the resulting object must remain
+ * valid: the failed canonize must leave the stack partially
+ * canonicalized but hole-free, so that inspecting it, retrying
+ * canonize, and freeing it are all safe. ASan / UBSan-instrumented
+ * builds will catch any double-free or use-after-free in those walks.
*/
static int test_asid_canonize_error_midsweep(void)
{
goto err;
/*
- * Successful return below relies on ASIdentifiers_free walking
- * the partially-compacted stack without UAF or double-free.
- * Under ASan / UBSan that walk is the actual test.
+ * The object must also be safe to inspect and to retry, not only
+ * to free: both calls walk (and the second re-sorts) the stack,
+ * so they would crash on any NULL slot left by the mid-sweep merge.
+ */
+ if (!TEST_int_eq(X509v3_asid_is_canonical(asid), 0)
+ || !TEST_int_eq(X509v3_asid_canonize(asid), 0))
+ goto err;
+
+ /*
+ * Successful return below relies on ASIdentifiers_free walking the
+ * partially-canonicalized, hole-free stack without UAF or
+ * double-free. Under ASan / UBSan that walk is the actual test.
*/
testresult = 1;
err:
* in IPAddressOrRanges_canonize. Construct a list whose first half is
* adjacent and mergeable, with a duplicate at position k that hits the
* overlap check after a series of merges has driven write < read.
- * The canonize call must return 0, and the family's normal teardown
- * (sk_IPAddressFamily_pop_free) must safely walk the partially
- * compacted stack -- ASan / UBSan catches any double-free or UAF the
- * mixed-state-on-error invariant would otherwise miss. Because the
- * v3_addr.c canonize uses direct `return 0` rather than a `done:`
- * cleanup label, the teardown invariant for this file is different
- * from the asid path and warrants its own coverage.
+ * The canonize call must return 0, and the resulting object must
+ * remain valid: the failed canonize must leave the stack partially
+ * canonicalized but hole-free, so that inspecting it, retrying
+ * canonize, and freeing it are all safe. ASan / UBSan catches any
+ * double-free or UAF in those walks.
*/
static int test_addr_canonize_error_midsweep(void)
{
if (!TEST_int_eq(X509v3_addr_canonize(addr), 0))
goto end;
+ /*
+ * The object must also be safe to inspect and to retry, not only
+ * to free: both calls walk (and the second re-sorts) the stack,
+ * so they would crash on any NULL slot left by the mid-sweep merge.
+ */
+ if (!TEST_int_eq(X509v3_addr_is_canonical(addr), 0)
+ || !TEST_int_eq(X509v3_addr_canonize(addr), 0))
+ goto end;
+
/*
* Successful return below relies on sk_IPAddressFamily_pop_free
- * walking the partially-compacted aors stack without UAF or
- * double-free. Under ASan / UBSan that walk is the actual test.
+ * walking the partially-canonicalized, hole-free aors stack
+ * without UAF or double-free. Under ASan / UBSan that walk is
+ * the actual test.
*/
testresult = 1;
end:
return testresult;
}
+/*
+ * Verify that an ASIdentifiers object remains safe to inspect and to
+ * retry after a canonize() call fails. The input [1, 2, 2] fails
+ * because 2 overlaps the merged [1, 2] range; the merge of 1 and 2
+ * runs first, so the failure happens mid-sweep. canonize() must
+ * return 0 and leave the object in a state where is_canonical() and
+ * a second canonize() both run without crashing and return 0.
+ */
+static int test_asid_canonize_failure_then_inspect(void)
+{
+ ASIdentifiers *asid = NULL;
+ ASN1_INTEGER *val = NULL;
+ int testresult = 0;
+
+ if (!TEST_ptr(asid = ASIdentifiers_new()))
+ goto err;
+
+ if (!TEST_ptr(val = ASN1_INTEGER_new())
+ || !TEST_true(ASN1_INTEGER_set_int64(val, 1))
+ || !TEST_true(X509v3_asid_add_id_or_range(asid, V3_ASID_ASNUM,
+ val, NULL)))
+ goto err;
+ val = NULL;
+ if (!TEST_ptr(val = ASN1_INTEGER_new())
+ || !TEST_true(ASN1_INTEGER_set_int64(val, 2))
+ || !TEST_true(X509v3_asid_add_id_or_range(asid, V3_ASID_ASNUM,
+ val, NULL)))
+ goto err;
+ val = NULL;
+ if (!TEST_ptr(val = ASN1_INTEGER_new())
+ || !TEST_true(ASN1_INTEGER_set_int64(val, 2))
+ || !TEST_true(X509v3_asid_add_id_or_range(asid, V3_ASID_ASNUM,
+ val, NULL)))
+ goto err;
+ val = NULL;
+
+ /* canonize must reject the overlap. */
+ if (!TEST_int_eq(X509v3_asid_canonize(asid), 0))
+ goto err;
+
+ /* The object must be safe to inspect and to retry after the failure. */
+ if (!TEST_int_eq(X509v3_asid_is_canonical(asid), 0))
+ goto err;
+ if (!TEST_int_eq(X509v3_asid_canonize(asid), 0))
+ goto err;
+
+ testresult = 1;
+err:
+ ASN1_INTEGER_free(val);
+ ASIdentifiers_free(asid);
+ return testresult;
+}
+
+/*
+ * Verify that an IPAddrBlocks object remains safe to inspect and to
+ * retry after a canonize() call fails. The input
+ * [1.0.0.0/32, 1.0.0.1/32, 1.0.0.1/32] fails because the third
+ * prefix overlaps the merged range of the first two; that merge runs
+ * first, so the failure happens mid-sweep. canonize() must return 0
+ * and leave the object in a state where is_canonical() and a second
+ * canonize() both run without crashing and return 0.
+ */
+static int test_addr_canonize_failure_then_inspect(void)
+{
+ IPAddrBlocks *addr = NULL;
+ unsigned char ip0[4] = { 1, 0, 0, 0 };
+ unsigned char ip1[4] = { 1, 0, 0, 1 };
+ int testresult = 0;
+
+ if (!TEST_ptr(addr = sk_IPAddressFamily_new_null()))
+ goto end;
+
+ if (!TEST_true(X509v3_addr_add_prefix(addr, IANA_AFI_IPV4, NULL,
+ ip0, 32))
+ || !TEST_true(X509v3_addr_add_prefix(addr, IANA_AFI_IPV4, NULL,
+ ip1, 32))
+ || !TEST_true(X509v3_addr_add_prefix(addr, IANA_AFI_IPV4, NULL,
+ ip1, 32)))
+ goto end;
+
+ /* canonize must reject the overlap. */
+ if (!TEST_int_eq(X509v3_addr_canonize(addr), 0))
+ goto end;
+
+ /* The object must be safe to inspect and to retry after the failure. */
+ if (!TEST_int_eq(X509v3_addr_is_canonical(addr), 0))
+ goto end;
+ if (!TEST_int_eq(X509v3_addr_canonize(addr), 0))
+ goto end;
+
+ testresult = 1;
+end:
+ sk_IPAddressFamily_pop_free(addr, IPAddressFamily_free);
+ return testresult;
+}
+
/*
* Exercise the merge arm where `cur` is itself a range (rather than a
* single integer), hitting the `case ASIdOrRange_range` detach branch
* well-formed adjacent integers; entry k is an explicitly inverted
* range (min = 1000, max = 100). X509v3_asid_add_id_or_range does
* not validate min <= max for ranges, so the bad entry is admitted
- * into the list, and canonize must detect it on the sweep. The
- * teardown under ASan / UBSan verifies that the early-exit path
- * leaves the asIdsOrRanges stack in a freeable state.
+ * into the list, and canonize must detect it on the sweep. Like the
+ * overlap case, the failure happens after earlier merges have run, so
+ * canonize must return 0 and leave the object safe to inspect, retry,
+ * and free.
*
* The addr-side counterpart of this branch (v3_addr.c:849) is not
* reachable through the public API: make_addressRange refuses to
if (!TEST_int_eq(X509v3_asid_canonize(asid), 0))
goto err;
+ /*
+ * The object must also be safe to inspect and to retry, not only
+ * to free: both calls walk (and the second re-sorts) the stack,
+ * so they would crash on any NULL slot left by the mid-sweep merge.
+ */
+ if (!TEST_int_eq(X509v3_asid_is_canonical(asid), 0)
+ || !TEST_int_eq(X509v3_asid_canonize(asid), 0))
+ goto err;
+
testresult = 1;
err:
ASN1_INTEGER_free(val);
ADD_TEST(test_addr_interleaved_canonize);
ADD_TEST(test_asid_canonize_error_midsweep);
ADD_TEST(test_addr_canonize_error_midsweep);
+ ADD_TEST(test_asid_canonize_failure_then_inspect);
+ ADD_TEST(test_addr_canonize_failure_then_inspect);
ADD_TEST(test_asid_range_merge_canonize);
ADD_TEST(test_asid_canonize_inverted_midsweep);
#endif /* OPENSSL_NO_RFC3779 */
projects / thirdparty / openssl.git / commitdiff
