Skip to content

bug: prevent nil-error panic in login failure paths #205

Description

@p3t3r67x0

Problem

The Login handler calls err.Error() in branches where err can be nil.

For example, after successful JSON binding:

  • if email or password is empty
  • err is normally nil
  • the code calls debugf(err.Error())

The same pattern occurs after a successful database query when:

  • the user is inactive
  • or password comparison fails

At this point the database query error is also nil, but the code again calls:

debugf(err.Error())

Calling Error() on a nil error interface can panic.

Gin recovery may prevent the whole process from terminating, but an ordinary invalid login request should never trigger a panic/recovery path.

Impact

Malformed or incorrect login attempts can:

  • generate HTTP 500 responses instead of clean 401 responses
  • create unnecessary panic/recovery logs
  • make monitoring noisy
  • potentially allow trivial denial-of-service amplification

Proposed changes

Do not reference err in branches unrelated to that error.

For example log explicit messages such as:

  • "missing login credentials"
  • "invalid email or password"
  • "inactive account"

Avoid logging sensitive credentials or passwords.

Also search the repository for similar patterns:

if condition { debugf(err.Error()) }

where err may be nil.

Acceptance criteria

  • Empty email/password does not panic
  • Incorrect password does not panic
  • Inactive account does not panic
  • Login failure consistently returns HTTP 401
  • No password or credential contents are logged
  • Regression tests cover all login failure branches

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions