9elements / converged-security-suite

Converged Security Suite for Intel & AMD platform security features
https://www.9esec.io
BSD 3-Clause "New" or "Revised" License
58 stars 15 forks source link

bg-prov: bpm-gen is panicking because of out of bounds array access #382

Open ansiwen opened 8 months ago

ansiwen commented 8 months ago

This code is essentially always crashing, because idx iterates over img.Segs, but ibbElements is only of size ibbCount, which is almost always smaller.

https://github.com/9elements/converged-security-suite/blob/8e176a0bb8493584a972805def838c7dfb344aee/pkg/provisioning/bootguard/bootguard.go#L1071-L1082

I could easily fix that, but first I want to have an conversation how that even could happen? Are there no tests at all covering this code?

I worry about the general state of the code, because I don't believe anyone coded it like that, but there rather was some incident like a corrupt merge/rebase, that probably broke a lot more code?

It got introduced by https://github.com/9elements/converged-security-suite/pull/355/commits/7df8824b2eab66f8550f3b78020d4de1324902b0 in #355 which indeed was rebased a couple of times.

@zaolin @walterchris

walterchris commented 7 months ago

I was kind of hopping that @zaolin would address this - however he does not. So let me pick it up. I found some time to rework some little things here and there, and can help to fix it up, and build proper testing.

I think when @zaolin was rewrite some code, that code pulled in without testing, and broke everything (as you pointed already out here).

As I said, I am happy to help fix it up.