Microsoft-authentication-library-for-js: Error in acquireTokenSilent caused by ServerTelemetryManager.maxErrorsToSend, and causing infinite loops

[Nick-Lucas]

Please follow the issue template below. Failure to do so will result in a delay in answering your question.

Library

• [X] @azure/[email protected]
• [X] @azure/[email protected]
• [ ] @azure/[email protected]

Framework

React

Description

To preface I believe the particularly nasty (infinite loops) version of this bug may be surfacing because of how we handle errors while acquiring tokens, but it looks to be a mistake in a recent refactor none-the-less, as this should probably not be surfacing an error in the public API.

While calling acquireTokenSilent we are seeing errors come back with the error: TypeError Cannot read propertylengthof null. This causes infinite loops and locks up our application.

I inserted the following logging into the function within node_modules which in the below screenshot shows many errors are stored as null but the MSAL code is totally un-safely accessing these.

if (!errorCode) console.log("LOG", serverTelemetryEntity.failedRequests, serverTelemetryEntity.errors)

Just changing the implementation to this is enough to prevent the error and avoid an infinite loop:

var errorCode = serverTelemetryEntity.errors[i] || '';

Error Message

msal.js:46 [AcquireTokenSilent] Unhandled Error:TypeError TypeError: Cannot read property 'length' of null
    at Function.ServerTelemetryManager.maxErrorsToSend (index.es.js:7220)
    at ServerTelemetryManager.generateLastRequestHeaderValue (index.es.js:7136)
    at RefreshTokenClient.BaseClient.createDefaultTokenRequestHeaders (index.es.js:4286)
    at RefreshTokenClient.<anonymous> (index.es.js:6357)
    at step (index.es.js:394)
    at Object.next (index.es.js:324)
    at fulfilled (index.es.js:276)

Security

• [ ] Is this issue security related?

Regression

• [X ] Did this behavior work before?

2.3.0 is the last stable version

MSAL Configuration

/**
 * @type {import("@azure/msal-browser").Configuration}
 */
export const config = {
  auth: {
    authority:
      window._env_.B2C_AUTHORITY + '/' + window._env_.B2C_SIGNIN_POLICY,
    knownAuthorities: [window._env_.B2C_AUTHORITY],
    clientId: window._env_.CLIENT_ID,
    redirectUri: location.origin + '/signin-oidc',
    navigateToLoginRequestUrl: true,
    postLogoutRedirectUri: location.origin + '/'
  },
  cache: {
    cacheLocation: 'localStorage',
    storeAuthStateInCookie: true
  },
  system: {
    iframeHashTimeout: 8000
  }
}

Reproduction steps

It appears to happen fairly randomly on page reloads, and is bringing down our e2e suite as a result. The fix appears fairly obvious so hopefully a complete reproduction is not required here.

Expected behavior

Browsers/Environment

• [X ] Chrome

[jessemartin]

+1
I was about to report this exact same issue.

[Nick-Lucas]

Okay so it looks like it was introduced in 2.3.1 by this PR:

https://github.com/AzureAD/microsoft-authentication-library-for-js/pull/2223/files#diff-8f520c4118a0debf84e7a09b00a8de21269592c97333980f204b7d99e1e8d3acR129

Just more safely accessing the error code length is enough to offset the problem, though I can imagine there being a truer root cause that error codes should never be null?

2.3.0 seems to be stable for now.

Exact problem lines:

https://github.com/AzureAD/microsoft-authentication-library-for-js/blob/618794a389a958745e6daed1b8d5174959369f1b/lib/msal-common/src/telemetry/server/ServerTelemetryManager.ts#L126-L129

[jessemartin]

The errorCodes used after the loop starts are undefined as they come from the TypeError that is thrown when trying to access errorCode.length, which gets casted to null when run through Storage.

[pkanher617]

@Nick-Lucas @jessemartin Thanks for looking into this. I will take a look and try to push a patch out ASAP. Also happy to accept a PR if you know what the fix is.

[tnorling]

@Nick-Lucas Thanks again for bringing this to our attention. I've just put up a PR which should fix this, you can expect it to be included in the next release, currently slated to be 2.4.1