Infinite loop when parsing bdb table

[daveharmon]

At https://github.com/knqyf263/go-rpmdb/blob/master/pkg/bdb/hash_page.go#L70

I believe this continue statement without any changes to the loop variables is causing an infinite loop for some types of corrupted bdb databases.

I am able to pretty consistently recreate this issue, though I am unable to give you an example at this time.

Adding "currentPageNo = currentPage.NextPageNo`" breaks the loop and causes this library to detect the corruption, though I'm not totally sure if that's the right solution. Please let me know.

[daveharmon]

It seems like the pages that are entering this if-statement are just fully zero'd-out, so their page type is 0

[daveharmon]

I would appreciate it if someone could ack this issue and say if they plan to take a look or if this is likely to linger until more consumers +1 it

[knqyf263]

Contributions welcome

[michaelvolovik]

Hello
Seems that we have a fix for this issue.
Will it be possible to create PR for approval and merge?

[knqyf263]

Sure

[rhdesmond]

We are seeing this issue and investigating

[rhdesmond]

Shouldn't this be part of the loop post statement? Otherwise the line mentioned by @daveharmon will cause an infinite loop.

Alternatively, we can break out of the loop when we see that the page isn't an overflow page (e.g. it's a new page, so we are done parsing the overflow page?), but I'm not 100% sure of the semantics here.

[rhdesmond]

Seems like Jfrog has forked the library with a timeout fix: https://github.com/jfrog/go-rpmdb

[knqyf263]

I need a contribution.

[rjammala]

@knqyf263 can you please accept the fix?

[knqyf263]

Honestly, I don't think adding timeout is ideal. We should detect the infinite loop somehow.

[rhdesmond]

The infinite loop is when it hits the currentPage.PageType != OverflowPageType condition, then continues without updating the loop variable or erroring out.

The reason I did not modify the hash table parsing is I don't know if this is expected behavior. If you think it's incorrect db file, we can just error when it hits

if currentPage.PageType != OverflowPageType { continue }

and change the continue to

return nil, xerrors.Errorf("Invalid page encountered when parsing hash page")

[rhdesmond]

The other option is to add the loop update clause to before the continue statement:

if currentPage.PageType != OverflowPageType {
  currentPageNo = currentPage.NextPageNo
  continue
}

[knqyf263]

@wagoodman Do you know something about it?
84f408f