fix(#913): guard CreateSmoothed's fixed-stride shapes array against a mismatched section edge count

Scripts/patches/0027-BRepOffsetAPI_ThruSections-CreateSmoothed-section-edge-count-guard-913.patch

@@ -0,0 +1,153 @@
+From: OCCTSwift ecosystem <noreply@secondmouse.au>
+Subject: [PATCH] Modeling Algorithms - guard CreateSmoothed's fixed-stride
+ shapes array against a mismatched section edge count
+
+BRepOffsetAPI_ThruSections::CreateSmoothed() derives `nbEdges` -- the edge
+count it assumes every section has -- from section 1 alone (or section 2, if
+section 1 is a punctual/degenerate vertex section). It then allocates
+`shapes`, an NCollection_Array1<TopoDS_Shape>, sized exactly
+`nbSects * nbEdges`, and fills it by walking each section's wire with a
+BRepTools_WireExplorer, incrementing a running index with no bounds check.
+
+Nothing enforces that every section actually has `nbEdges` edges.
+BRepFill_CompatibleWires (via CheckCompatibility(true), the default)
+normally reconciles differing edge counts across sections before
+CreateSmoothed() ever runs. With CheckCompatibility(false) ("no check"),
+that reconciliation is skipped entirely, so a later section with a
+DIFFERENT edge count than section 1 either:
+
+  - has MORE edges: the fill loop walks past the end of `shapes` -- an
+    out-of-bounds write, observed as heap corruption and a later SIGSEGV
+    inside CreateSmoothed() itself once at least 3 sections are involved (2
+    sections always take the CreateRuled() path instead, which does not
+    share this allocation shape); or
+  - has FEWER edges: no overrun (the total write count can still land inside
+    `shapes`' bounds), so Build() reports success today -- but with
+    per-section strides silently misaligned to different geometry than
+    intended, an incorrect result with no signal anything went wrong.
+
+Confirmed via a minimal, from-scratch C++ reproducer with no OCCTSwift/
+bridge involvement: two matching circle sections build successfully,
+reusing the same builder with a third, differently-shaped section (more
+edges than the first two) and rebuilding SIGSEGVs on the stock library.
+Needs no reused builder as such for the overrun case -- what actually
+varies is process/allocator state at the time of the overrun, which is why
+an otherwise-identical single Build() call with all three sections added up
+front does not reliably crash even though the same out-of-bounds write
+still occurs. The fewer-edges case does not depend on process state at all:
+confirmed directly that a 2/1/3-edge three-section input under
+CheckCompatibility(false) reports Build() == true and Shape().IsValid() ==
+false against the stock library, every time.
+
+Fix: before allocating `shapes`, walk every non-punctual section and count
+its edges; if any section's count differs from `nbEdges`, set myStatus to
+BRepFill_ThruSectionErrorStatus_ProfilesInconsistent and return, matching
+the early-return idiom this function already uses two lines below
+(TS.IsNull() -> myStatus = Failed; return;). This is symmetric by
+construction (an inequality test, not a "too many" test): it also rejects
+the previously-silent fewer-edges case, which is intentional -- both
+directions are the same underlying contract violation, only one of them
+used to crash.
+
+Punctual end sections (a degenerate vertex added via AddVertex(), e.g. a
+cone's apex) are exempt, matching the existing w1Point/w2Point handling
+throughout the rest of the function -- a point section legitimately has a
+different edge count and is not itself walked by the fill loop's "punctual"
+branch the same way. That branch repeats the section's one degenerate edge
+nbEdges times to fill its slots; it does not merely skip validation, so a
+punctual section still needs at least that one edge to exist. w1Point/
+w2Point are computed by an existing loop that is vacuously true for a wire
+with NO edges at all (the loop body that would clear it never runs), which
+would let the fill loop below walk an uninitialized BRepTools_WireExplorer
+and read a null edge -- not reachable through any wrapper this project
+ships (a wire needs at least one edge to exist there), but the guard added
+here checks for at least one edge before exempting a section as punctual,
+rather than trusting w1Point/w2Point's classification unconditionally.
+
+The guard shares its punctual-section test (`isPunctualSection`, a local
+lambda) with the pre-existing fill loop 15 lines below, which used to
+duplicate the same two-clause boolean expression separately -- hoisted so
+the two cannot silently drift apart on which sections take the punctual
+branch.
+
+Validated by override-linking the patched translation unit ahead of the
+stock archive (both with and without No_Exception/NDEBUG, matching the
+production Release configuration this project ships) across six scenarios:
+the three that legitimately succeed today (checkCompatibility(false) with
+genuinely matching section edge counts; a punctual section at either end
+mixed with matching wire sections; checkCompatibility(true) with mismatched
+sections, reconciled by BRepFill_CompatibleWires as before) are all
+byte-for-byte unaffected; the original crashing scenario (more edges) now
+fails cleanly (IsDone() == false) instead of crashing; a new fewer-edges
+scenario, silently "successful" with invalid geometry on the stock library,
+now also fails cleanly; and a section with genuinely zero edges at the
+punctual position no longer reaches the fill loop's unguarded walk.
+
+SecondMouseAU/OCCTSwift#913.
+---
+ .../TKOffset/BRepOffsetAPI/BRepOffsetAPI_ThruSections.cxx    | 46 +++++++++++++++++++++++++++++++++++++++++++++-
+ 1 file changed, 45 insertions(+), 1 deletion(-)
+
+--- a/src/ModelingAlgorithms/TKOffset/BRepOffsetAPI/BRepOffsetAPI_ThruSections.cxx
++++ b/src/ModelingAlgorithms/TKOffset/BRepOffsetAPI/BRepOffsetAPI_ThruSections.cxx
+@@ -747,6 +760,51 @@ void BRepOffsetAPI_ThruSections::CreateSmoothed()
+     }
+   }
+
++  // #913: nbEdges above comes from section 1 (or 2, if punctual) alone. CheckCompatibility(false)
++  // skips BRepFill_CompatibleWires, so nothing else guarantees every other section has the same
++  // edge count -- and the fixed-stride fill loop below indexes `shapes` on that assumption with no
++  // bounds check, overrunning it (heap corruption, observed as a later SIGSEGV) for a section with
++  // more edges than section 1, or silently misaligning per-section strides (no crash, but a wrong
++  // result) for one with fewer. Refuse cleanly instead. Shared with the fill loop's own identical
++  // test below, so the two can't drift apart on which sections take the punctual branch.
++  auto isPunctualSection = [&](int theIndex) {
++    return (theIndex == 1 && w1Point) || (theIndex == nbSects && w2Point);
++  };
++  for (int iSect = 1; iSect <= nbSects; iSect++)
++  {
++    if (isPunctualSection(iSect))
++    {
++      // A punctual end section (AddVertex()) has exactly ONE degenerate edge, which the fill
++      // loop below repeats nbEdges times to fill that section's slots -- not zero edges. w1Point/
++      // w2Point are also (vacuously) true for a wire with NO edges at all, which the fill loop
++      // would then walk with an uninitialized BRepTools_WireExplorer, reading a null edge. Not
++      // reachable through any wrapper this project ships (a wire needs at least one edge to
++      // exist), but a punctual classification should not admit that case either.
++      bool hasAnyEdge = false;
++      for (anExp.Init(TopoDS::Wire(myWires(iSect))); anExp.More(); anExp.Next())
++      {
++        hasAnyEdge = true;
++        break;
++      }
++      if (!hasAnyEdge)
++      {
++        myStatus = BRepFill_ThruSectionErrorStatus_ProfilesInconsistent;
++        return;
++      }
++      continue;
++    }
++    int aSectEdges = 0;
++    for (anExp.Init(TopoDS::Wire(myWires(iSect))); anExp.More(); anExp.Next())
++    {
++      aSectEdges++;
++    }
++    if (aSectEdges != nbEdges)
++    {
++      myStatus = BRepFill_ThruSectionErrorStatus_ProfilesInconsistent;
++      return;
++    }
++  }
++
+   // recover the shapes
+   bool                             uClosed = true;
+   NCollection_Array1<TopoDS_Shape> shapes(1, nbSects * nbEdges);
+@@ -765,7 +823,7 @@ void BRepOffsetAPI_ThruSections::CreateSmoothed()
+         uClosed = false;
+       }
+     }
+-    if ((i == 1 && w1Point) || (i == nbSects && w2Point))
++    if (isPunctualSection(i))
+     {
+       // if the wire is punctual
+       anExp.Init(TopoDS::Wire(wire));

Scripts/patches/README.md

@@ -765,6 +765,115 @@ baked in (`Package.swift`'s own census: "ALL FIFTEEN ARE VERIFIED PRESENT IN THE
 `0026` is carried on disk only — the first patch since `0022`-`0025` themselves were folded into
 that release to sit outside the pin. Watch for it at the next kernel re-pin.
 
+## 0027-BRepOffsetAPI_ThruSections-CreateSmoothed-section-edge-count-guard-913.patch
+
+**Fixes the upstream OCCT defect behind [#913](https://github.com/SecondMouseAU/OCCTSwift/issues/913)**
+— found incidentally while investigating #910 (a different, bridge-side #905-review finding):
+`ThruSectionsBuilder`, given `checkCompatibility(false)` and a section whose edge count differs
+from the first section, SIGSEGVs instead of failing cleanly.
+
+`CreateSmoothed()` derives `nbEdges` — the edge count it assumes every section has — from section 1
+alone (or section 2, if section 1 is a punctual/degenerate vertex section). It allocates `shapes`,
+an `NCollection_Array1<TopoDS_Shape>` sized exactly `nbSects * nbEdges`, then fills it by walking
+each section's wire with a `BRepTools_WireExplorer`, incrementing a running index with **no bounds
+check**. Nothing enforces that every section actually has `nbEdges` edges: `BRepFill_CompatibleWires`
+(via `checkCompatibility(true)`, the default) normally reconciles differing edge counts across
+sections before `CreateSmoothed()` ever runs, but `checkCompatibility(false)` skips that entirely.
+A section with a **different** edge count than section 1 goes one of two ways:
+
+- **More edges**: the fill loop walks the array straight past the end of `shapes` — an
+  out-of-bounds write, corrupting adjacent heap memory rather than raising a catchable failure.
+- **Fewer edges**: no overrun (the total write count can still land inside `shapes`' bounds), so
+  `Build()` reports success today — but with per-section strides silently misaligned to different
+  geometry than intended. Confirmed directly: a 2/1/3-edge three-section input under
+  `checkCompatibility(false)` reports `Build() == true` and `Shape().IsValid() == false` against
+  the stock library, deterministically, no process-state dependency (PR #915 review finding 3 —
+  the first version of this patch's own SemVer note undersold this, describing only the crashing
+  direction).
+
+**Reached only with 3+ sections.** With exactly 2 sections, `Build()` always takes the
+`CreateRuled()` path instead (`if (myWires.Length() == 2 || myIsRuled) CreateRuled(); else
+CreateSmoothed();`), which builds its shell via `BRepFill_Generator` — a different mechanism that
+doesn't share this fixed-stride allocation. A single `Build()` call with all mismatched (more-edges)
+sections present from the start does **not** reliably crash even though the identical out-of-bounds
+write still occurs; what actually varies is process/allocator state at the time of the overrun
+(confirmed directly: instrumenting the fill loop shows the write index reaching one past
+`shapes.Upper()` either way, but a from-scratch process quietly lands the write in
+unmapped-but-harmless heap slack where a process that already ran one successful `Build()` call does
+not). Symptom, not cause: this made the defect look like it needed a *reused* builder when it
+doesn't — any 3+-section `checkCompatibility(false)` call with mismatched edge counts carries the
+same latent corruption (or, in the fewer-edges direction, the same silent misalignment,
+deterministically regardless of process state).
+
+**Fix:** before allocating `shapes`, walk every non-punctual section and count its edges; on a
+mismatch (an inequality test, not a "too many" test — deliberately symmetric, since both directions
+are the same underlying contract violation and only one of them used to crash), set `myStatus` to
+`BRepFill_ThruSectionErrorStatus_ProfilesInconsistent` and return, matching the early-return idiom
+this function already uses two lines below (`TS.IsNull()` -> `myStatus = Failed; return;`). Punctual
+end sections (`AddVertex()`, e.g. a cone's apex) are exempt, matching the existing
+`w1Point`/`w2Point` handling throughout the rest of the function — a point section legitimately has
+a different edge count and the fill loop's punctual branch doesn't walk it the same way: that branch
+repeats the section's own edge `nbEdges` times to fill its slots, so it needs at least one edge to
+exist, not zero (PR #915 review finding 11 — an earlier draft of this entry, and the patch's own
+first-draft comment, said "(zero) edge count"; `AddVertex()` creates a wire with exactly **one**
+degenerate edge). `w1Point`/`w2Point` themselves are computed by a pre-existing loop that is
+vacuously `true` for a wire with no edges at all — not reachable through any wrapper this project
+ships (a `Wire` needs at least one edge to exist), but the guard checks for at least one edge before
+exempting a section as punctual rather than trusting that classification unconditionally (PR #915
+review finding 4; attempted to reproduce a live crash for this specific case via both a fresh and a
+reused builder and could not — OCCT's own pipeline handles a genuinely empty wire more gracefully
+than the code reading alone suggested — but the added check is correct and cheap regardless of
+whether today's fill loop actually reaches the unguarded path it describes).
+
+The guard shares its punctual-section test (`isPunctualSection`, a local lambda) with the
+pre-existing fill loop 15 lines below, which used to duplicate the same two-clause boolean
+expression separately (PR #915 review finding 9) — hoisted so the two cannot silently drift apart on
+which sections take the punctual branch.
+
+**Validation** (override-link, no full rebuild for this patch's own writeup — see `#0001`'s retired
+entry for the technique; the OCCTSwift-side PR carrying this one also rebuilds the local
+xcframework, since #913 asked for that explicitly rather than deferring it like `0026`): compiled
+and linked both the unpatched and patched `.cxx` ahead of the pinned `libOCCT-macos.a`, across six
+scenarios — the three that legitimately succeed today (matching edge counts under
+`checkCompatibility(false)`; a punctual section at either end mixed with matching wire sections;
+`checkCompatibility(true)` with mismatched sections, reconciled by `BRepFill_CompatibleWires` as
+before) are all byte-for-byte unaffected; the original more-edges scenario now fails cleanly
+(`IsDone() == false`) instead of crashing; the new fewer-edges scenario, silently "successful" with
+invalid geometry on the stock library, now also fails cleanly; and a section with genuinely zero
+edges at the punctual position no longer reaches the fill loop's unguarded walk. Re-confirmed all
+six hold with `No_Exception`/`NDEBUG` defined (matching a Release build configuration close to what
+the shipped archive itself was likely built with): the fix eliminates the out-of-bounds write
+itself, so it does not depend on `Standard_OutOfRange`'s range check being compiled in.
+`clang-format --dry-run --Werror` against OCCT's own `.clang-format` reports zero violations on
+both changed files.
+
+**The zero-edge-at-punctual-position scenario is a code hardening, not a proven-live fix.**
+Committed only the fewer-edges GTest, not a zero-edge one: the fewer-edges scenario genuinely fails
+`EXPECT_FALSE(IsDone())` against the pristine, unpatched source (proved first, per this project's
+own "prove the test fails" convention, before writing the fix), but a from-scratch equivalent for a
+genuinely empty wire at the punctual position passes *even against pristine, unpatched code* —
+something else downstream (most likely `TotalSurf()`'s own null-surface guard, a few lines below,
+reacting to whatever a degenerate empty-wire input produces) already reports `IsDone() == false`
+for it today, by a different and unconfirmed mechanism, independent of this patch. The explicit
+`hasAnyEdge` check is kept anyway (it is correct regardless, and cheap), but no committed test
+claims to be proof it closes a live gap, because the one written could not be made to fail without
+it — see finding 4 in the PR #915 review thread for the full attempt, including a reused-builder
+variant that also did not reproduce a crash.
+
+**SIGSEGV vs. SIGBUS** (PR #915 review finding 12): both signals were genuinely observed for the
+more-edges overrun, in different binaries, and that is not a contradiction to reconcile down to one
+— it's exactly what heap corruption looks like. The standalone from-scratch C++ reproducer (its own
+`backtrace_symbols_fd`-based signal handler installed) reported signal 11 (SIGSEGV) consistently.
+The upstream GTest (`MismatchedSectionEdgeCountFailsCleanlyWithoutCheck`, linked against the stock
+archive with no custom handler, OS default handling) reported signal 10 (SIGBUS). Same defect, same
+out-of-bounds write, two different binaries with different allocator/memory layout at the moment of
+the overrun — which of the two manifests is exactly the kind of detail this defect's own root-cause
+section says depends on process/allocator state, not something either transcript got wrong.
+
+Filed upstream as [OCCT#1466](https://github.com/Open-Cascade-SAS/OCCT/pull/1466).
+
+**Retire** once the bundled OCCT includes this fix.
+
 # Retired patches
 
 The `.patch` files below are **deleted**. Each fix now comes from the pinned OCCT release itself, so