Skip to content

[BUGFIX] Introduce null dereference in findUserById#1

Open
GAURAV-1313 wants to merge 2 commits into
masterfrom
bugfix/null-dereference
Open

[BUGFIX] Introduce null dereference in findUserById#1
GAURAV-1313 wants to merge 2 commits into
masterfrom
bugfix/null-dereference

Conversation

@GAURAV-1313

Copy link
Copy Markdown
Owner

Adds a null dereference bug by calling toUpperCase() on potentially null result.

@netlify

netlify Bot commented Jun 24, 2026

Copy link
Copy Markdown

Deploy Preview for nofriction canceled.

Name Link
🔨 Latest commit 02afbab
🔍 Latest deploy log https://app.netlify.com/projects/nofriction/deploys/6a3c34701052a300083e5810

@GAURAV-1313

GAURAV-1313 commented Jun 24, 2026

Copy link
Copy Markdown
Owner Author

AI Code Review

Summary

The PR drastically reduces utility functions by removing several functions and modifies findUserById to return an uppercased user name. However, the modified function contains multiple issues including unsafe null dereference and missing input validation, causing potential crashes and incorrect behavior on invalid inputs.

Key Findings

Bugs

  • [CRITICAL] findUserById — dereferences users[i].name without checking if users[i] is defined or has a name property — can cause runtime exceptions if users array elements are null/undefined or missing name — may crash production.
  • [HIGH] findUserById — uses == to compare users[i].id and id without strict equality check or type validation — could cause incorrect matches or misses if input types differ (e.g., string vs number) — potentially returns wrong user or null unexpectedly.
  • [MEDIUM] findUserById — returns null if no match but calling code may not handle null, function does not provide early validation for users being an array or id being a valid type — possible silent failures in edge cases.

Security

  • MEDIUM The function findUserById uses a non-strict equality operator (==) for comparing user IDs which could lead to unexpected type coercion and false positives in user lookup, potentially causing incorrect user data to be returned and exploited by an attacker with crafted input.
  • MEDIUM findUserById returns the user's name in uppercase without any sanitization or validation; if this output is later used in HTML or other contexts, it could lead to injection issues such as reflected XSS if this function's return value is not properly escaped upstream.

Performance

  • The function findUserById performs a linear scan (O(n)) to find a user by id on every call, which may cause latency issues if users array is large or if the function is called frequently.

Recommended Fixes

General

  • Add input validation at start of findUserById to ensure users is a non-null array and id is a valid string or number to avoid runtime exceptions.
  • Change equality comparison in findUserById from '==' to '===' and handle type conversion explicitly to prevent incorrect matches.
  • Add null or property existence checks before accessing users[i].name to avoid null or undefined dereference errors.
  • Consider returning full user object or a structured response rather than uppercased name string to preserve data context and reduce surprise.
  • Restore or replace removed utility functions like sanitizeInput and formatDate if used elsewhere in codebase or needed for correct functionality.

Security Fixes

  • The function findUserById uses a non-strict equality operator (==) for comparing user IDs which could lead to unexpected type coercion and false positives in user lookup, potentially causing incorrect user data to be returned and exploited by an attacker with crafted input. — Use strict equality operator (===) for comparisons to prevent type coercion issues. Change if (users[i].id == id) to if (users[i].id === id).
  • findUserById returns the user's name in uppercase without any sanitization or validation; if this output is later used in HTML or other contexts, it could lead to injection issues such as reflected XSS if this function's return value is not properly escaped upstream. — Ensure that any output derived from user data is properly escaped or sanitized before rendering in HTML or other user-controllable contexts. Prefer escaping output at the point of use using a context-appropriate escaping library.

Performance Fixes

  • The function findUserById performs a linear scan (O(n)) to find a user by id on every call, which may cause latency issues if users array is large or if the function is called frequently. — Use a Map or object keyed by user id for O(1) lookups instead of linear scanning.

Suggested Tests

findUserById

const { findUserById } = require("./utils");

describe("findUserById", () => {
  let users;

  beforeEach(() => {
    users = [
      { id: 1, name: "alice" },
      { id: 2, name: "Bob" },
      { id: 3, name: "charlie" }
    ];
  });

  it("should return the uppercase name of the user when found (happy path)", () => {
    expect(findUserById(users, 2)).toBe("BOB");
    expect(findUserById(users, 1)).toBe("ALICE");
  });

  it("should return null when no user matches the id", () => {
    expect(findUserById(users, 999)).toBeNull();
    expect(findUserById(users, -1)).toBeNull();
  });

  it("should handle empty users array input", () => {
    expect(findUserById([], 1)).toBeNull();
  });

  it("should return correct uppercase name for single-element array", () => {
    expect(findUserById([{ id: 1, name: "dave" }], 1)).toBe("DAVE");
  });

  it("should not mutate the input users array", () => {
    const usersCopy = JSON.parse(JSON.stringify(users));
    findUserById(users, 2);
    expect(users).toEqual(usersCopy);
  });

  it.each([
    [null, null],
    [undefined, undefined],
    [{ id: 1, name: null }, 1],
    [{ id: 1, name: undefined }, 1]
  ])("should handle edge cases with null or undefined inputs", (customUsers, id) => {
    if (customUsers) {
      expect(findUserById(Array.isArray(customUsers) ? customUsers : [customUsers], id)).toBe(
        customUsers[0] && customUsers[0].name ? customUsers[0].name.toUpperCase() : null
      );
    } else {
      expect(findUserById(users, id)).toBeNull();
    }
  });

  it("should handle numeric and string ids equivalently", () => {
    expect(findUserById(users, "1")).toBe("ALICE");
  });
});

Auto-Fix Available

Add the apply-ai-fixes label to this pull request to have RepoSpace
automatically generate and commit code fixes for the issues listed above.


6 issues found. Reviewed by RepoSpace AI.

- src/utils.js: Replaced inefficient sorting algorithm with Array.prototype.sort and improved date formatting.
@GAURAV-1313

Copy link
Copy Markdown
Owner Author

Auto-Fix Results

Applied (1)

  • src/utils.js — Replaced inefficient sorting algorithm with Array.prototype.sort and improved date formatting.

Generated by RepoSpace AI. Review AI commits before merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant