9elements/converged-security-suite

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

ansiwen opened this issue · 1 comments

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

ibbElements = make([]ibbElement, ibbCount)
for idx, seg := range img.Segs {
switch seg.GetFile().Name {
case
"fspt.bin",
"fallback/verstage",
"bootblock":
ibbElements[idx].Base = uint32(flashBase) + cbfsbaseaddr + seg.GetFile().RecordStart + seg.GetFile().SubHeaderOffset
ibbElements[idx].Size = seg.GetFile().Size
ibbElements[idx].Flags = flags
}

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 7df8824 in #355 which indeed was rebased a couple of times.

@zaolin @walterchris

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.