-
-
Notifications
You must be signed in to change notification settings - Fork 99
Description
Hello 👋🏻
I use libopenapi in some projects and while implementing some fuzz tests for an unrelated bug, I came across a case where libopenapi itself panics.
Minimal Reproduction
package main
import (
"fmt"
"os"
"github.com/pb33f/libopenapi"
)
func main() {
doc, _ := libopenapi.NewDocument([]byte("<<"))
_, err := doc.BuildV3Model() // panics: index out of range [1] with length 1
if err != nil {
fmt.Fprintf(os.Stderr, "Error: %v\n", err)
}
}Stack Trace
✦ ❯ go run main.go
panic: runtime error: index out of range [1] with length 1
goroutine 1 [running]:
github.com/pb33f/libopenapi/utils.FindKeyNode({0x10268f884, 0x7}, {0x7c0f31612110?, 0x102c2fec0?, 0x7c0f316560c0?})
/Users/tomfleet/go/pkg/mod/github.com/pb33f/libopenapi@v0.34.3/utils/utils.go:391 +0x2f8
github.com/pb33f/libopenapi/datamodel.extractSpecInfoInternal({0x7c0f31610974, 0x2, 0x2}, 0x0, 0x0)
/Users/tomfleet/go/pkg/mod/github.com/pb33f/libopenapi@v0.34.3/datamodel/spec_info.go:130 +0x3b0
github.com/pb33f/libopenapi/datamodel.ExtractSpecInfoWithDocumentCheck(...)
/Users/tomfleet/go/pkg/mod/github.com/pb33f/libopenapi@v0.34.3/datamodel/spec_info.go:76
github.com/pb33f/libopenapi.NewDocumentWithTypeCheck({0x7c0f31610974?, 0x1022cf240?, 0x7c0f31520601?}, 0x0?)
/Users/tomfleet/go/pkg/mod/github.com/pb33f/libopenapi@v0.34.3/document.go:147 +0x28
github.com/pb33f/libopenapi.NewDocument(...)
/Users/tomfleet/go/pkg/mod/github.com/pb33f/libopenapi@v0.34.3/document.go:143
main.main()
/Users/tomfleet/Development/scratch/libopenapi-panic/main.go:11 +0x3c
exit status 2
I think this is
Lines 390 to 392 in 4012566
| if nodes != nil && len(nodes) > 0 && nodes[0].Tag == "!!merge" { | |
| nodes = NodeAlias(nodes[1]).Content | |
| } |
Which I think should instead check for len(nodes) > 1 in this case. I understand this is invalid YAML so the underlying yaml parser should probably reject it, but instead it seems to just create 1 !!merge node.
I didn't do too much deep diving on the knock-on effects of this but I've tested this out against a local version of libopenapi with the above change applied and can confirm it fixes the issue and doesn't fail any other tests but you will likely have some additional insights/ideas/concerns :)
✦ ❯ go run main.go
Error: failed to decode YAML to JSON: yaml: construct errors:
line 1: cannot construct !!merge `<<` into map[string]interface {}
exit status 1
Anyway, let me know if you would like a PR for this fix. I appreciate it's an edge case and technically invalid YAML etc. so no worries if not but it surfaced for me in a fuzz test so I thought you might be interested in seeing it 🫡