BUG REPORT: Wrong pointer in as_check_overlap() — TAL constraints bypass
=========================================================================
Component: rpki-client
File: usr.sbin/rpki-client/as.c, lines 75-76
Version: $OpenBSD: as.c,v 1.17 2024/11/12 09:23:07 tb Exp $
Severity: Medium (CVSS 5.4)
Category: Logic error — overlap detection failure
1. DESCRIPTION
--------------
In as_check_overlap(), the CERT_AS_RANGE / CERT_AS_ID case at lines 75-76
uses the wrong pointer. The code reads:
case CERT_AS_RANGE:
if (ases->range.min > ases[i].id || /* BUG */
ases->range.max < ases[i].id) /* BUG */
continue;
break;
The function signature is:
int
as_check_overlap(const struct cert_as *as, const char *fn,
const struct cert_as *ases, size_t num_ases, int quiet)
Where:
- `as` = the newly-parsed incoming AS entry being checked
- `ases` = the existing array of previously accepted AS entries
`ases->range.min` is equivalent to `ases[0].range.min` — it accesses the
first element of the existing array, NOT the incoming entry. The correct
code should use `as->range.min` and `as->range.max`.
Every other branch in the function correctly uses `as->` for the incoming
entry:
- Line 71: `as->id` (CERT_AS_ID / CERT_AS_ID case)
- Line 86: `as->id` (CERT_AS_RANGE / CERT_AS_ID case)
- Line 91: `as->range.max` (CERT_AS_RANGE / CERT_AS_RANGE case)
- Line 92: `as->range.min` (CERT_AS_RANGE / CERT_AS_RANGE case)
- Lines 75-76: `ases->range.min`, `ases->range.max` *** WRONG ***
The struct cert_as is a union (extern.h lines 51-57):
struct cert_as {
enum cert_as_type type;
union {
uint32_t id;
struct cert_as_range range;
};
};
When ases[0] is a CERT_AS_ID singleton (type == CERT_AS_ID), reading
ases[0].range.min and ases[0].range.max accesses the union as the wrong
member. Since calloc zero-initializes memory and the `id` field occupies
only the first uint32_t of the union, `range.max` reads zero. This makes
the overlap check evaluate to "no overlap" for any incoming range where
range.min > 0 or range.max < 0 (i.e., almost always).
IMPACT:
as_check_overlap() is called from two sites:
1. cert.c:1278 — during certificate parsing, to detect internally
overlapping AS resources (RFC 3779 section 3.2.3.4 violation).
2. constraints.c:532 — during TAL constraint checking, to test whether
a certificate's AS numbers overlap with a deny-list.
For the constraints path: if a deny rule is a singleton AS (CERT_AS_ID)
and sits at position [0] in the deny array, and a CA operator presents a
certificate with an AS range (CERT_AS_RANGE) covering that singleton, the
overlap check fails to detect the match. The deny rule is silently
bypassed, allowing unauthorized AS numbers to pass RPKI validation.
This can enable BGP route origin hijacking for operators who rely on TAL
.constraints files to restrict which AS numbers are considered valid.
2. PROOF OF CONCEPT
--------------------
The bug can be demonstrated by tracing as_check_overlap() through the
constraints path. No special tooling is required — the logic error is
visible by inspection. Below is a concrete scenario:
Setup:
- TAL operator configures a .constraints file that denies AS 64496
(a singleton CERT_AS_ID entry in the deny array)
- deny_ases[0] = { .type = CERT_AS_ID, .id = 64496 }
Attack:
- Rogue CA presents a certificate with an AS range covering 64496,
e.g., range [64490, 64500]
- cert_as = { .type = CERT_AS_RANGE, .range = { .min=64490, .max=64500 } }
Expected behavior:
- as_check_overlap() should detect that 64496 falls within [64490, 64500]
- Function should return 0 (overlap detected), blocking the certificate
Actual behavior with the bug:
- Code enters the outer switch case CERT_AS_ID (ases[i].type == CERT_AS_ID)
- Code enters the inner switch case CERT_AS_RANGE (as->type == CERT_AS_RANGE)
- Code evaluates:
if (ases->range.min > ases[i].id || ases->range.max < ases[i].id)
which is:
if (ases[0].range.min > ases[0].id || ases[0].range.max < ases[0].id)
Since ases[0] IS the deny entry with type CERT_AS_ID:
ases[0].range.min = ases[0].id = 64496 (same union storage)
ases[0].range.max = 0 (zero-init padding past id)
Evaluates to:
if (64496 > 64496 || 0 < 64496)
if (false || true)
-> continue (no overlap detected!)
- as_check_overlap() returns 1 (no overlap), incorrectly allowing the cert
To reproduce programmatically, compile and run the standalone
poc-ob051-as-overlap.c (included alongside this report):
$ cc -Wall -o poc-ob051 poc-ob051-as-overlap.c
$ ./poc-ob051
Output:
=== OB-051 POC: as_check_overlap() wrong pointer ===
Test 1: Range [64490-64500] vs deny singleton AS 64496
BUGGY result: 1 (1=no overlap, 0=overlap)
FIXED result: 0 (1=no overlap, 0=overlap)
Expected: 0 (overlap — range covers the singleton)
>>> BUG CONFIRMED: buggy code missed the overlap!
>>> Fixed code correctly detects the overlap.
Test 2: Range [65000-65000] vs deny singleton AS 65000
BUGGY result: 1
FIXED result: 0
Expected: 0 (overlap — exact match)
>>> BUG CONFIRMED: buggy code missed the overlap!
>>> Fixed code correctly detects the overlap.
Test 3: Range [65000-65100] vs deny singleton AS 64496 (no overlap)
BUGGY result: 1
FIXED result: 1
Expected: 1 (no overlap)
>>> Both correctly report no overlap.
Test 4: Range [195-205] vs deny array [AS100, AS200]
BUGGY result: 1
FIXED result: 0
Expected: 0 (overlap with AS 200)
>>> BUG CONFIRMED: buggy code missed the overlap!
>>> (It compared ases[0].range against ases[i].id
>>> instead of as->range against ases[i].id)
>>> Fixed code correctly detects the overlap.
=== RESULT: BUG DEMONSTRATED ===
The bug is confirmed in all overlap scenarios. The fixed version
correctly detects all overlaps while preserving correct behavior
for the non-overlapping case (Test 3).
3. SUGGESTED FIX
-----------------
Change `ases->` to `as->` on lines 75 and 76 of as.c:
--- a/usr.sbin/rpki-client/as.c
+++ b/usr.sbin/rpki-client/as.c
@@ -72,8 +72,8 @@ as_check_overlap(const struct cert_as *as, const char *fn,
if (as->id != ases[i].id)
continue;
break;
case CERT_AS_RANGE:
- if (ases->range.min > ases[i].id ||
- ases->range.max < ases[i].id)
+ if (as->range.min > ases[i].id ||
+ as->range.max < ases[i].id)
continue;
break;
This makes the CERT_AS_ID/CERT_AS_RANGE branch consistent with every other
branch in the function, which all use `as->` to reference the incoming entry.
