diff --git a/fuzz/oss-fuzz-build.sh b/fuzz/oss-fuzz-build.sh index 2ef9552cc3586..596dabd1d9c00 100755 --- a/fuzz/oss-fuzz-build.sh +++ b/fuzz/oss-fuzz-build.sh @@ -123,7 +123,10 @@ build_teleport_fuzzers() { FuzzParseU2FRegistrationResponse fuzz_parse_u2f_registration_response compile_native_go_fuzzer $TELEPORT_PREFIX/lib/web \ - FuzzTdpMFACodecDecode fuzz_tdp_mfa_codec_decode + FuzzTdpMFACodecDecodeChallenge fuzz_tdp_mfa_codec_decode_challenge + + compile_native_go_fuzzer $TELEPORT_PREFIX/lib/web \ + FuzzTdpMFACodecDecodeResponse fuzz_tdp_mfa_codec_decode_response compile_native_go_fuzzer $TELEPORT_PREFIX/lib/multiplexer \ FuzzReadProxyLineV1 fuzz_read_proxy_linec_v1 diff --git a/lib/auth/http_client.go b/lib/auth/http_client.go index a9850ceaa17f3..8eeac3bb141b0 100644 --- a/lib/auth/http_client.go +++ b/lib/auth/http_client.go @@ -780,7 +780,7 @@ func (c *HTTPClient) ValidateOIDCAuthCallback(ctx context.Context, q url.Values) if err != nil { return nil, trace.Wrap(err) } - var rawResponse *OIDCAuthRawResponse + var rawResponse OIDCAuthRawResponse if err := json.Unmarshal(out.Bytes(), &rawResponse); err != nil { return nil, trace.Wrap(err) } @@ -818,7 +818,7 @@ func (c *HTTPClient) ValidateSAMLResponse(ctx context.Context, re string, connec if err != nil { return nil, trace.Wrap(err) } - var rawResponse *SAMLAuthRawResponse + var rawResponse SAMLAuthRawResponse if err := json.Unmarshal(out.Bytes(), &rawResponse); err != nil { return nil, trace.Wrap(err) } diff --git a/lib/auth/trustedcluster.go b/lib/auth/trustedcluster.go index 9d85a0463b4e1..296c77d3d43fe 100644 --- a/lib/auth/trustedcluster.go +++ b/lib/auth/trustedcluster.go @@ -654,7 +654,7 @@ func (a *Server) sendValidateRequestToProxy(host string, validateRequest *Valida return nil, trace.Wrap(err) } - var validateResponseRaw *ValidateTrustedClusterResponseRaw + var validateResponseRaw ValidateTrustedClusterResponseRaw err = json.Unmarshal(out.Bytes(), &validateResponseRaw) if err != nil { return nil, trace.Wrap(err) diff --git a/lib/client/redirect.go b/lib/client/redirect.go index 53f270c8c24ac..80b4114d52bc9 100644 --- a/lib/client/redirect.go +++ b/lib/client/redirect.go @@ -204,13 +204,13 @@ func (rd *Redirector) issueSSOLoginConsoleRequest(req SSOLoginConsoleReq) (*SSOL return nil, trace.Wrap(err) } - var re *SSOLoginConsoleResponse + var re SSOLoginConsoleResponse err = json.Unmarshal(out.Bytes(), &re) if err != nil { return nil, trace.Wrap(err) } - return re, nil + return &re, nil } // Done is called when redirector is closed @@ -255,13 +255,13 @@ func (rd *Redirector) callback(w http.ResponseWriter, r *http.Request) (*auth.SS return nil, trace.BadParameter("failed to decrypt response: in %v, err: %v", r.URL.String(), err) } - var re *auth.SSHLoginResponse + var re auth.SSHLoginResponse err = json.Unmarshal(plaintext, &re) if err != nil { return nil, trace.BadParameter("failed to decrypt response: in %v, err: %v", r.URL.String(), err) } - return re, nil + return &re, nil } // Close closes redirector and releases all resources diff --git a/lib/client/weblogin.go b/lib/client/weblogin.go index bec0807bcb403..b52b679ade062 100644 --- a/lib/client/weblogin.go +++ b/lib/client/weblogin.go @@ -464,13 +464,13 @@ func SSHAgentLogin(ctx context.Context, login SSHLoginDirect) (*auth.SSHLoginRes return nil, trace.Wrap(err) } - var out *auth.SSHLoginResponse + var out auth.SSHLoginResponse err = json.Unmarshal(re.Bytes(), &out) if err != nil { return nil, trace.Wrap(err) } - return out, nil + return &out, nil } // SSHAgentHeadlessLogin begins the headless login ceremony, returning new user certificates if successful. @@ -497,13 +497,13 @@ func SSHAgentHeadlessLogin(ctx context.Context, login SSHLoginHeadless) (*auth.S return nil, trace.Wrap(err) } - var out *auth.SSHLoginResponse + var out auth.SSHLoginResponse err = json.Unmarshal(re.Bytes(), &out) if err != nil { return nil, trace.Wrap(err) } - return out, nil + return &out, nil } // SSHAgentPasswordlessLogin requests a passwordless MFA challenge via the proxy. diff --git a/lib/srv/desktop/tdp/proto.go b/lib/srv/desktop/tdp/proto.go index 6ddfc105f746f..399f4f0181fb6 100644 --- a/lib/srv/desktop/tdp/proto.go +++ b/lib/srv/desktop/tdp/proto.go @@ -596,6 +596,8 @@ func DecodeMFA(in byteReader) (*MFA, error) { if length > maxMFADataLength { _, _ = io.CopyN(io.Discard, in, int64(length)) return nil, mfaDataMaxLenErr + } else if length == 0 { + return nil, trace.BadParameter("mfa data missing") } b := make([]byte, int(length)) @@ -636,6 +638,8 @@ func DecodeMFAChallenge(in byteReader) (*MFA, error) { if length > maxMFADataLength { return nil, trace.BadParameter("mfa challenge data exceeds maximum length") + } else if length == 0 { + return nil, trace.BadParameter("mfa challenge data missing") } b := make([]byte, int(length)) @@ -643,17 +647,14 @@ func DecodeMFAChallenge(in byteReader) (*MFA, error) { return nil, trace.Wrap(err) } - var req *client.MFAAuthenticateChallenge + var req client.MFAAuthenticateChallenge if err := json.Unmarshal(b, &req); err != nil { return nil, trace.Wrap(err) } - if err != nil { - return nil, trace.Wrap(err) - } return &MFA{ Type: mt, - MFAAuthenticateChallenge: req, + MFAAuthenticateChallenge: &req, }, nil } diff --git a/lib/web/apiserver.go b/lib/web/apiserver.go index a04c9fa724396..ce84d5db529a2 100644 --- a/lib/web/apiserver.go +++ b/lib/web/apiserver.go @@ -2800,7 +2800,7 @@ func (h *Handler) siteNodeConnect( if params == "" { return nil, trace.BadParameter("missing params") } - var req *TerminalRequest + var req TerminalRequest if err := json.Unmarshal([]byte(params), &req); err != nil { return nil, trace.Wrap(err) } @@ -2824,13 +2824,13 @@ func (h *Handler) siteNodeConnect( clusterName := site.GetName() if req.SessionID.IsZero() { // An existing session ID was not provided so we need to create a new one. - sessionData, err = h.generateSession(ctx, clt, req, clusterName, sessionCtx) + sessionData, err = h.generateSession(ctx, clt, &req, clusterName, sessionCtx) if err != nil { h.log.WithError(err).Debug("Unable to generate new ssh session.") return nil, trace.Wrap(err) } } else { - sessionData, tracker, err = h.fetchExistingSession(ctx, clt, req, clusterName) + sessionData, tracker, err = h.fetchExistingSession(ctx, clt, &req, clusterName) if err != nil { return nil, trace.Wrap(err) } diff --git a/lib/web/command.go b/lib/web/command.go index 8e8662a6aba83..81d46cf341fbe 100644 --- a/lib/web/command.go +++ b/lib/web/command.go @@ -134,7 +134,7 @@ func (h *Handler) executeCommand( if params == "" { return nil, trace.BadParameter("missing params") } - var req *CommandRequest + var req CommandRequest if err := json.Unmarshal([]byte(params), &req); err != nil { return nil, trace.BadParameter("failed to read JSON message: %v", err) } diff --git a/lib/web/fuzz_test.go b/lib/web/fuzz_test.go index f421f6faeb28d..a14ac0df971d5 100644 --- a/lib/web/fuzz_test.go +++ b/lib/web/fuzz_test.go @@ -17,13 +17,87 @@ limitations under the License. package web import ( + "bytes" + "encoding/binary" + "encoding/json" + "math" "testing" "github.com/stretchr/testify/require" + + apiproto "github.com/gravitational/teleport/api/client/proto" + wanpb "github.com/gravitational/teleport/api/types/webauthn" + "github.com/gravitational/teleport/lib/defaults" + "github.com/gravitational/teleport/lib/srv/desktop/tdp" ) -func FuzzTdpMFACodecDecode(f *testing.F) { - f.Add([]byte("")) +func FuzzTdpMFACodecDecodeChallenge(f *testing.F) { + allowedCreds := wanpb.CredentialDescriptor{ + Type: "public-key", + Id: []byte{0x02, 0x02, 0x02, 0x02}, + } + extensions := wanpb.AuthenticationExtensionsClientInputs{AppId: "id"} + jsonData, err := json.Marshal(&apiproto.MFAAuthenticateChallenge{ + WebauthnChallenge: &wanpb.CredentialAssertion{ + PublicKey: &wanpb.PublicKeyCredentialRequestOptions{ + Challenge: []byte{0xAA, 0xAA, 0xAA, 0xAA}, + TimeoutMs: int64(120), + RpId: "RelyingPartyID", + AllowCredentials: []*wanpb.CredentialDescriptor{&allowedCreds}, + Extensions: &extensions, + UserVerification: "verification", + }, + }, + }) + require.NoError(f, err) + var normalBuf bytes.Buffer + var maxSizeBuf bytes.Buffer + // add initial bytes for protocol + _, err = normalBuf.Write([]byte{byte(tdp.TypeMFA), []byte(defaults.WebsocketWebauthnChallenge)[0]}) + require.NoError(f, err) + _, err = maxSizeBuf.Write([]byte{byte(tdp.TypeMFA), []byte(defaults.WebsocketWebauthnChallenge)[0]}) + require.NoError(f, err) + // Write the length using BigEndian encoding + require.NoError(f, binary.Write(&normalBuf, binary.BigEndian, uint32(len(jsonData)))) + require.NoError(f, binary.Write(&maxSizeBuf, binary.BigEndian, uint32(math.MaxUint32))) + // Write the JSON data itself + _, err = normalBuf.Write(jsonData) + require.NoError(f, err) + _, err = maxSizeBuf.Write(jsonData) + require.NoError(f, err) + + f.Add(normalBuf.Bytes()) + f.Add(maxSizeBuf.Bytes()) + f.Add([]byte{0xa, 0x6e, 0x0, 0x0, 0x0, 0x4, 0x6e, 0x75, 0x6c, 0x6c}) // nil json unmarshal without error + + f.Fuzz(func(t *testing.T, buf []byte) { + require.NotPanics(t, func() { + codec := tdpMFACodec{} + _, _ = codec.decodeChallenge(buf, "") + }) + }) +} + +func FuzzTdpMFACodecDecodeResponse(f *testing.F) { + var normalBuf bytes.Buffer + var maxSizeBuf bytes.Buffer + // add initial bytes for protocol + _, err := normalBuf.Write([]byte{byte(tdp.TypeMFA), []byte(defaults.WebsocketWebauthnChallenge)[0]}) + require.NoError(f, err) + _, err = maxSizeBuf.Write([]byte{byte(tdp.TypeMFA), []byte(defaults.WebsocketWebauthnChallenge)[0]}) + require.NoError(f, err) + mfaData := []byte("fake-data") + // Write the length using BigEndian encoding + require.NoError(f, binary.Write(&normalBuf, binary.BigEndian, uint32(len(mfaData)))) + require.NoError(f, binary.Write(&maxSizeBuf, binary.BigEndian, uint32(math.MaxUint32))) + // add data field + _, err = normalBuf.Write(mfaData) + require.NoError(f, err) + _, err = maxSizeBuf.Write(mfaData) + require.NoError(f, err) + + f.Add(normalBuf.Bytes()) + f.Add(maxSizeBuf.Bytes()) f.Fuzz(func(t *testing.T, buf []byte) { require.NotPanics(t, func() { diff --git a/lib/web/session/cookie.go b/lib/web/session/cookie.go index c3fbb24a6e919..a5deeae166246 100644 --- a/lib/web/session/cookie.go +++ b/lib/web/session/cookie.go @@ -46,11 +46,11 @@ func DecodeCookie(b string) (*Cookie, error) { if err != nil { return nil, err } - var c *Cookie + var c Cookie if err := json.Unmarshal(bytes, &c); err != nil { return nil, err } - return c, nil + return &c, nil } // SetCookie encodes the provided user and session id via [EncodeCookie]