diff --git a/internal/scan/openapi.go b/internal/scan/openapi.go index 179b49f..298b183 100644 --- a/internal/scan/openapi.go +++ b/internal/scan/openapi.go @@ -79,12 +79,21 @@ const ( // openapiSpec is the minimal slice of an openapi/swagger document we care about: // the version banner, info block, top-level security and the path map. unknown // fields are ignored by both json and yaml decoders. +// +// path items decode into a bare interface{} rather than a typed operation struct +// because openapi 3.1 allows a path item to be a "$ref" to a shared item instead +// of a set of operations. a strongly typed sibling map (map[string]rawOpsStruct) +// makes both json and yaml fail the whole document the moment one path item is a +// $ref string next to another path item with real get/post operations, which +// silently drops an otherwise valid, enumerable spec. interface{} accepts either +// shape without erroring, and operationSecurity below sorts out what's actually +// an operation object. type openapiSpec struct { - OpenAPI string `json:"openapi" yaml:"openapi"` - Swagger string `json:"swagger" yaml:"swagger"` - Info openapiInfo `json:"info" yaml:"info"` - Security []map[string][]string `json:"security" yaml:"security"` - Paths map[string]map[string]rawOps `json:"paths" yaml:"paths"` + OpenAPI string `json:"openapi" yaml:"openapi"` + Swagger string `json:"swagger" yaml:"swagger"` + Info openapiInfo `json:"info" yaml:"info"` + Security []map[string][]string `json:"security" yaml:"security"` + Paths map[string]map[string]interface{} `json:"paths" yaml:"paths"` } type openapiInfo struct { @@ -92,10 +101,58 @@ type openapiInfo struct { Version string `json:"version" yaml:"version"` } -// rawOps captures the per-operation security block. a pointer so an absent block -// (inherit global) is distinct from an explicit empty one (security: [] = public). -type rawOps struct { - Security *[]map[string][]string `json:"security" yaml:"security"` +// operationSecurity pulls the per-operation security block out of a decoded path +// item entry. it reports present=false when the entry isn't an operation object +// at all (a $ref path item, or a security key that was never declared), which the +// caller treats as "inherit global" the same as an absent security key. +func operationSecurity(op interface{}) (reqs []map[string][]string, present bool) { + obj, ok := op.(map[string]interface{}) + if !ok { + return nil, false + } + raw, ok := obj["security"] + if !ok { + return nil, false + } + // a security key whose value isn't a list (null, or a malformed scalar or + // object) is not a usable requirement block. treat it as absent and inherit + // the global default rather than fabricate an anonymous high-severity finding + // from garbage, which also matches how the old typed decoder handled a null. + if _, ok := raw.([]interface{}); !ok { + return nil, false + } + return toSecurityReqs(raw), true +} + +// toSecurityReqs converts a decoded "security" value (a list of scheme->scopes +// requirement objects) into the typed form securityAllowsAnonymous expects. +// malformed entries are skipped rather than treated as a parse failure, since by +// this point the document has already passed the openapi/swagger version check. +func toSecurityReqs(raw interface{}) []map[string][]string { + arr, ok := raw.([]interface{}) + if !ok { + return nil + } + reqs := make([]map[string][]string, 0, len(arr)) + for _, item := range arr { + obj, ok := item.(map[string]interface{}) + if !ok { + continue + } + req := make(map[string][]string, len(obj)) + for scheme, scopesRaw := range obj { + scopesArr, _ := scopesRaw.([]interface{}) + scopes := make([]string, 0, len(scopesArr)) + for _, s := range scopesArr { + if str, ok := s.(string); ok { + scopes = append(scopes, str) + } + } + req[scheme] = scopes + } + reqs = append(reqs, req) + } + return reqs } // OpenAPI probes the candidate spec paths concurrently and, on the first hit, @@ -294,8 +351,8 @@ func specToResult(spec *openapiSpec) *OpenAPIResult { } // an explicit block decides on its own; an absent one inherits global. var unauth bool - if op.Security != nil { - unauth = securityAllowsAnonymous(*op.Security) + if reqs, present := operationSecurity(op); present { + unauth = securityAllowsAnonymous(reqs) } else { unauth = globalAllowsAnon } diff --git a/internal/scan/openapi_test.go b/internal/scan/openapi_test.go index d532a61..4239806 100644 --- a/internal/scan/openapi_test.go +++ b/internal/scan/openapi_test.go @@ -268,6 +268,94 @@ func TestOpenAPI_YAMLSpec(t *testing.T) { } } +// a spec with a $ref path item (a shared item defined elsewhere, valid since +// openapi 3.1) sitting next to a normal operation. /pet can't be resolved without +// fetching the referenced document, but /users must still be enumerated: the +// $ref entry must not fail the whole document's decode. +const openapiJSONWithRef = `{ + "openapi": "3.1.0", + "info": {"title": "Ref API", "version": "1.0"}, + "paths": { + "/pet": {"$ref": "#/components/pathItems/Pet"}, + "/users": {"get": {"summary": "list"}} + } +}` + +// TestOpenAPI_RefPathItemDoesNotDropSpec locks a real regression: the old +// map[string]rawOps decoded every path item as a strict operation-set struct, so +// a $ref path item (its value is a plain string, not an object) failed both the +// json and yaml unmarshal for the *entire* document, and an otherwise valid, +// enumerable spec was silently rejected as "not a spec". +func TestOpenAPI_RefPathItemDoesNotDropSpec(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/openapi.json" { + _, _ = w.Write([]byte(openapiJSONWithRef)) + return + } + w.WriteHeader(http.StatusNotFound) + })) + defer srv.Close() + + result, err := OpenAPI(srv.URL, 5*time.Second, 4, "") + if err != nil { + t.Fatalf("OpenAPI: %v", err) + } + if result == nil { + t.Fatal("expected a result despite the $ref path item, got nil") + } + if _, ok := hasEndpoint(result, "/users", http.MethodGet); !ok { + t.Errorf("expected /users GET to be enumerated, got %+v", result.Endpoints) + } +} + +// a globally-secured spec whose operation carries a security key that isn't a +// list (here null). the interface{} extraction must treat this as absent and let +// the operation inherit the enforced global requirement, exactly as the old typed +// decoder did, not read it as an empty (public) block and fabricate a high finding. +const openapiJSONNullOpSecurity = `{ + "openapi": "3.0.1", + "info": {"title": "Null Sec API", "version": "1.0"}, + "security": [{"bearerAuth": []}], + "paths": { + "/weird": {"get": {"summary": "malformed security", "security": null}} + } +}` + +// TestOpenAPI_NonListOpSecurityInheritsGlobal locks the empty-vs-absent boundary +// against a false positive: a non-array security value must inherit the global +// requirement (authenticated, medium), not decode to an empty block (anonymous, +// high). the pre-interface{} struct decoded null to a nil pointer and inherited; +// the extraction has to preserve that or every malformed security key becomes a +// spurious high-severity unauthenticated finding. +func TestOpenAPI_NonListOpSecurityInheritsGlobal(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/openapi.json" { + _, _ = w.Write([]byte(openapiJSONNullOpSecurity)) + return + } + w.WriteHeader(http.StatusNotFound) + })) + defer srv.Close() + + result, err := OpenAPI(srv.URL, 5*time.Second, 4, "") + if err != nil { + t.Fatalf("OpenAPI: %v", err) + } + if result == nil { + t.Fatal("expected a result, got nil") + } + ep, ok := hasEndpoint(result, "/weird", http.MethodGet) + if !ok { + t.Fatal("expected /weird GET to be enumerated") + } + if ep.Unauth { + t.Error("a non-list security value should inherit the global requirement, not read as public") + } + if result.Severity != openapiSevMedium { + t.Errorf("expected medium severity, got %q", result.Severity) + } +} + // TestOpenAPI_NoSpecExposed confirms a server with no spec at any candidate path // produces no result. func TestOpenAPI_NoSpecExposed(t *testing.T) {