Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add CodeQL suppressions for false positives #904

Merged
merged 1 commit into from
Feb 5, 2025
Merged

Conversation

Avery-Dunn
Copy link
Collaborator

Fixes the CodeQL issues from #899:

  • Three issues in the samples folder: suppressed since they are only meant to help manual testing during development, and the code is not included in any released package
  • Two issues AcquireTokenByInteractiveFlowSupplier: the issue was String concatenation with user input, however the URL is validated earlier in the interactive flow
  • One issue in DefaultHttpClientManagedIdentity: the issue was setting a HostnameVerifier that accepts all host names, however the TrustManager that uses the verifier only accepts a specific certificate thumbprint (this code was copied from the Azure Identity Java SDK and I discussed this behavior with them)

@Avery-Dunn Avery-Dunn requested a review from a team as a code owner February 5, 2025 16:26
@Avery-Dunn Avery-Dunn merged commit ffaa5d2 into dev Feb 5, 2025
5 checks passed
@@ -32,6 +32,7 @@ static void removeStateNonceCookies(HttpServletResponse httpResponse){
Cookie stateCookie = new Cookie(MSAL_WEB_APP_STATE_COOKIE, "");
stateCookie.setMaxAge(0);

// CodeQL [java/insecure-cookie]: Suppressing CodeQL warning since this is just a sample
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Avery-Dunn - can we revisit this? Our samples should not use insecure practices.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just a simpler version of the "real" sample meant to help with manual E2E testing rather than teach something, however it turns out the same code is in the real sample: https://github.com/Azure-Samples/ms-identity-msal-java-samples/blob/main/1-server-side/msal-b2c-web-sample/src/main/java/com/microsoft/azure/msalwebsample/CookieHelper.java#L30

I've just created an issue about auditing the samples to find these sorts of insecure practices: #911

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants