Crash (objectForKeyedSubscript: on NSArray) when the token endpoint returns a JSON array body

Cerrado
  • ¿Algo no está bien?
#987 1 comentario 0 reacciones 0 asignados Ver en GitHub

Los mantenedores suelen responder en 4 días

Ya se ha fusionado un pull request relacionado.

  • #988 de @w-goog — fusionado

Evaluación

Dificultad
3/5
Tiempo estimado
1-2 días
Aptitud para principiantes
68/100
Tipo de issue
Error
Claridad
Bien especificado
Estado de actividad
Activo
Stack tecnológico
objective-c, swift
Área
backend-api-design

Línea de trabajo

Start at +[OIDAuthorizationService performTokenRequest:originalAuthorizationResponse:callback:], then inspect the registration path performRegistrationRequest:completion: and +[OIDFieldMapping remainingParametersWithMap:parameters:instance:]. Check how each path handles parsed JSON that is not a dictionary, and look for tests covering token and registration responses with array bodies. Done means malformed response bodies return callback errors without terminating the app, while existing object responses retain their expected behavior.

Escrito por el modelo de indexación a partir del texto del issue.

Descripción

Describe the bug
+[OIDAuthorizationService performTokenRequest:originalAuthorizationResponse:callback:] assumes the token endpoint's JSON body is an object. If the body is valid JSON but an array, the app terminates with an uncaught NSInvalidArgumentException instead of the callback receiving an error:

  • HTTP 4xx with an array body (for example [] or ["error"]): the 4xx branch parses the body with NSJSONSerialization and reads json[OIDOAuthErrorFieldError], which sends objectForKeyedSubscript: to an NSArray.
  • HTTP 200 with a non-empty array body (for example ["x"]): the array is passed to -[OIDTokenResponse initWithRequest:parameters:], and +[OIDFieldMapping remainingParametersWithMap:parameters:instance:] sends objectForKeyedSubscript: to it.
  • HTTP 200 with []: no crash, but the empty array is accepted and the callback gets a non-nil OIDTokenResponse with no error.

From reading the code, the registration path (performRegistrationRequest:completion:) has the same pattern: its HTTP 400 branch subscripts the parsed JSON, and its success path passes it to OIDRegistrationResponse, which also goes through remainingParametersWithMap:. I have not run a repro for registration.

An authorization server should never answer this way, but a proxy, gateway or CDN in front of it can, and a crash is a heavy failure mode for a malformed response. #863 looks like the same crash (there the server sent nested arrays); it was closed as an IdP problem. This report is about the library terminating the app rather than returning an error.

*** Terminating app due to uncaught exception 'NSInvalidArgumentException', reason: '-[__NSArray0 objectForKeyedSubscript:]: unrecognized selector sent to instance ...'
	5   Repro   __86+[OIDAuthorizationService performTokenRequest:originalAuthorizationResponse:callback:]_block_invoke + 788

*** Terminating app due to uncaught exception 'NSInvalidArgumentException', reason: '-[__NSSingleObjectArrayI objectForKeyedSubscript:]: unrecognized selector sent to instance ...'
	5   Repro   +[OIDFieldMapping remainingParametersWithMap:parameters:instance:] + 320
	6   Repro   -[OIDTokenResponse initWithRequest:parameters:] + 244
	7   Repro   __86+[OIDAuthorizationService performTokenRequest:originalAuthorizationResponse:callback:]_block_invoke + 2140

To Reproduce
Self-contained, no network: a URLProtocol installed through OIDURLSessionProvider.setSession answers the token request. Build as a macOS command-line target that depends on AppAuthCore, then run it with a status code and a body, for example Repro 400 '[]'.

import AppAuthCore
import Foundation

final class StubProtocol: URLProtocol {
    static var status = Int(CommandLine.arguments[1])!
    static var body = Data(CommandLine.arguments[2].utf8)
    override class func canInit(with request: URLRequest) -> Bool { true }
    override class func canonicalRequest(for request: URLRequest) -> URLRequest { request }
    override func startLoading() {
        let response = HTTPURLResponse(url: request.url!, statusCode: Self.status, httpVersion: "HTTP/1.1",
                                       headerFields: ["Content-Type": "application/json"])!
        client?.urlProtocol(self, didReceive: response, cacheStoragePolicy: .notAllowed)
        client?.urlProtocol(self, didLoad: Self.body)
        client?.urlProtocolDidFinishLoading(self)
    }
    override func stopLoading() {}
}

let configuration = URLSessionConfiguration.ephemeral
configuration.protocolClasses = [StubProtocol.self]
OIDURLSessionProvider.setSession(URLSession(configuration: configuration))
let service = OIDServiceConfiguration(
    authorizationEndpoint: URL(string: "https://issuer.example.invalid/authorize")!,
    tokenEndpoint: URL(string: "https://issuer.example.invalid/token")!)
let request = OIDTokenRequest(
    configuration: service, grantType: OIDGrantTypeRefreshToken, authorizationCode: nil,
    redirectURL: nil, clientID: "example-client", clientSecret: nil, scope: nil,
    refreshToken: "example-refresh-token", codeVerifier: nil, additionalParameters: nil)
OIDAuthorizationService.perform(request) { response, error in
    print("callback response=\(response != nil) error=\(String(describing: error))")
    exit(0)
}
RunLoop.main.run(until: Date().addingTimeInterval(10))

Results against master at 812a9ac1 (the same on the 3.0.0 tag, a972daac):

Status Body Result
400 [] crash, -[__NSArray0 objectForKeyedSubscript:]
400 ["proxy error"] crash, -[__NSSingleObjectArrayI objectForKeyedSubscript:]
400 {"error":"invalid_grant"} callback, org.openid.appauth.oauth_token -10 (expected)
502 [] callback, org.openid.appauth.general -6 (expected; the body is only parsed for 4xx)
200 ["x"] crash in OIDFieldMapping remainingParametersWithMap:
200 [] callback with a non-nil OIDTokenResponse and no error

Expected behavior
No crash. For a 4xx whose body is not a JSON object, the callback gets OIDErrorCodeServerError, as it does today for a 4xx with a non-JSON body. For a 200 whose body is not a JSON object, the callback gets OIDErrorCodeJSONDeserializationError (or OIDErrorCodeTokenResponseConstructionError).

A possible fix: check [json isKindOfClass:[NSDictionary class]] before subscripting in the token 4xx branch and the registration 400 branch, and treat a non-dictionary success body as a deserialization error in both success paths. -[OIDServiceDiscovery initWithJSONData:error:] already does a similar check for discovery documents.

Screenshots
N/A (console output above).

Environment

  • Device: Mac mini (Apple silicon)
  • OS: macOS 27.0.1, Swift 6.4; the code paths are shared with iOS (AppAuthCore)
  • Browser: N/A (token endpoint request, no browser involved)
  • AppAuth-iOS: 3.0.0 and master (812a9ac1), via Swift Package Manager
Lenguaje dominante
Objective-C
Estrellas
2k
Forks
863
Merge medio
7 d 55 min
PR fusionados (30 d)
7

Preparar el entorno

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de openid/AppAuth-iOS

Todos los issues de openid/AppAuth-iOS

Issues similares

Más issues de Objective-C

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.