OWASP / OWASP/SecurityShepherd

authUserSSO result array: result[3] unset, result[5] double-assigned, wrong log message

Open
#820 0 comments 0 reactions 1 assignee View on GitHub

@ismisepaul is already working on this.

Since Apr 1, 2026.

Bug
Dominant language
Java
Stars
1.5k
Forks
515
Avg merge
3h 46m
Merged PRs (30d)
1

Description

Problem

Getter.authUserSSO() has two copy-paste bugs from authUser() that affect the result array contract:

1. result[5] assigned twice — first value silently lost
// Getter.java lines 454-456
result[5] = "false"; // sso logins can't change password
result[4] = classId; // classId
result[5] = Boolean.toString(isTempUsername); // overwrites the line above

The first assignment ("false") is immediately overwritten by isTempUsername. The comment suggests the intent was to set a "can't change password" flag at index 3 (the tempPassword slot), not index 5.

2. result[3] (tempPassword flag) never set — left as null

The authUser() method sets all 6 indices:

// authUser contract (Getter.java lines 158-166)
result[0] = userId
result[1] = userName
result[2] = role
result[3] = Boolean.toString(tempPassword)  // ← set in authUser
result[4] = classId
result[5] = Boolean.toString(tempUsername)

authUserSSO() skips result[3] entirely. Any caller that reads result[3] (e.g. Boolean.parseBoolean(result[3])) will get false by accident (parseBoolean treats null as false), but any caller that does result[3].equals(...) will NPE.

3. Log message says "End authUser" instead of "End authUserSSO"
// Getter.java line 458
log.debug("$$$ End authUser $$$"); // should be authUserSSO

Misleading when debugging auth flows in logs.

Proposed fix

result[0] = userID;
result[1] = userName;
result[2] = userRole;
result[3] = "false"; // tempPassword — SSO logins can't change password
result[4] = classId;
result[5] = Boolean.toString(isTempUsername);

log.debug("$$$ End authUserSSO $$$");

This aligns authUserSSO with the same result array contract as authUser.

Risk

Low — the fix is mechanical (reorder assignments, fix log message). No logic change, no schema change. But it touches a security-sensitive auth path, so it should be reviewed carefully and tested with SSO login flow if available.

Context

Identified by Copilot code review on PR #817 (connection leak fixes). Deferred from that PR to avoid scope creep in a pooling/leak-fix change.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.