From a727c2ebd36866b08f693f7be944792f330dbe95 Mon Sep 17 00:00:00 2001 From: Fredrik Bagge Carlson Date: Fri, 15 May 2026 19:39:22 +0200 Subject: [PATCH 1/2] Guard out-of-bounds accesses in SisoZpk minreal The pole-zero cancellation loop accessed `newZ[zidx+1]` (assuming the matched complex zero has a conjugate partner immediately after it) and `sys.p[pidx]` after `pidx += 1` (assuming another pole exists) without bounds checks. Both raise `BoundsError` on malformed inputs (or inputs that become so after earlier `deleteat!` operations). Add a `zidx < length(newZ)` check before the conjugate-cleanup, and break out of the loop after incrementing `pidx` past the available poles (still removing the matched zero for consistency). Co-Authored-By: Claude Opus 4.7 (1M context) --- lib/ControlSystemsBase/src/types/SisoTfTypes/SisoZpk.jl | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/ControlSystemsBase/src/types/SisoTfTypes/SisoZpk.jl b/lib/ControlSystemsBase/src/types/SisoTfTypes/SisoZpk.jl index 91ec7d191..9e3f87f54 100644 --- a/lib/ControlSystemsBase/src/types/SisoTfTypes/SisoZpk.jl +++ b/lib/ControlSystemsBase/src/types/SisoTfTypes/SisoZpk.jl @@ -79,11 +79,12 @@ function minreal(sys::SisoZpk{T,TR}, eps::Real) where {T, TR} distance, zidx = findmin(abs.(p .- newZ)) if distance < eps - if imag(p) == 0 && imag(newZ[zidx]) != 0 + if imag(p) == 0 && imag(newZ[zidx]) != 0 && zidx < length(newZ) newZ[zidx+1] = real(newZ[zidx+1]) end if imag(newZ[zidx]) == 0 && imag(p) != 0 pidx += 1 + pidx > length(sys.p) && (deleteat!(newZ, zidx); break) p = real(sys.p[pidx]) deleteat!(newZ, zidx) continue From 925eaa5008cd713f8e95cd941de46eb44e81e2ff Mon Sep 17 00:00:00 2001 From: Fredrik Bagge Carlson Date: Sat, 16 May 2026 06:10:10 +0200 Subject: [PATCH 2/2] Add regression tests for SisoZpk minreal bounds fix Construct complex-valued zpk inputs that exercise both unguarded accesses in the cancellation loop (lines 83 and 87 of SisoZpk.jl): one where a real pole matches a -imag complex zero whose conjugate partner was already deleted (newZ[zidx+1] out of bounds), and one where a complex pole at the last index is increment-matched past the end of sys.p. Both inputs threw BoundsError on master and now return the fully cancelled `zpk([], [], k)`. Co-Authored-By: Claude Opus 4.7 (1M context) --- lib/ControlSystemsBase/test/test_zpk.jl | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lib/ControlSystemsBase/test/test_zpk.jl b/lib/ControlSystemsBase/test/test_zpk.jl index 955eb5cbc..94fb10ea7 100644 --- a/lib/ControlSystemsBase/test/test_zpk.jl +++ b/lib/ControlSystemsBase/test/test_zpk.jl @@ -144,6 +144,14 @@ k = 0.3 @test minreal(zpk([-1.0, -2.0], Float64[], 2.5)) == zpk([-1.0, -2.0], Float64[], 2.5) @test minreal(zpk(Float64[], [-1.0, -2.0], 2.5)) == zpk(Float64[], [-1.0, -2.0], 2.5) +# Regression: bounds-checks in SisoZpk minreal. With complex-valued zpk the +# constructor does not enforce conjugate-pair ordering, so the cancellation +# loop can hit `newZ[zidx+1]` (line 83) with `zidx == length(newZ)`, or +# advance `pidx` past `length(sys.p)` before reading `sys.p[pidx]` (line 87). +# Both previously threw BoundsError; they should now return cleanly. +@test minreal(zpk([1.0+0.001im, 1.0-0.001im], [1.0, 1.0+0.001im], 1.0+0im), 0.01) == zpk(ComplexF64[], ComplexF64[], 1.0+0im) +@test minreal(zpk([1.0+0.001im, 1.0-0.001im], [1.0+0.001im, 1.0], 1.0+0im), 0.01) == zpk(ComplexF64[], ComplexF64[], 1.0+0im) + # Test type inference @test eltype(fill(zpk("s"),2)) <: TransferFunction @test eltype(fill(zpk([1],[1,1],1),2)) <: TransferFunction