Skip to content

fix invalid memory address or nil pointer dereference in baseClient.initConn#3676

Merged
ndyakov merged 11 commits intoredis:masterfrom
olde-ducke:fix-init-conn
Mar 23, 2026
Merged

fix invalid memory address or nil pointer dereference in baseClient.initConn#3676
ndyakov merged 11 commits intoredis:masterfrom
olde-ducke:fix-init-conn

Conversation

@olde-ducke
Copy link
Copy Markdown
Contributor

@olde-ducke olde-ducke commented Jan 14, 2026

Resolves #3675.

Fixes nil pointer dereference and potential deadlock in *baseClient.initConn().


Note

Low Risk
Low risk: adds nil-argument panics to client constructors and updates Options() docs; behavior only changes for previously invalid nil inputs and for callers mutating returned option pointers.

Overview
Adds explicit nil option guards (panic with clear message) to NewClusterClient and documents the same expectation across NewClient, NewRing, NewFailoverClient, NewFailoverClusterClient, NewSentinelClient, and NewUniversalClient.

Clarifies that Client.Options(), ClusterClient.Options(), and Ring.Options() return read-only option pointers and that mutating the returned structs may cause undefined behavior.

Written by Cursor Bugbot for commit 3694c07. This will update automatically on new commits. Configure here.

@jit-ci
Copy link
Copy Markdown

jit-ci Bot commented Jan 14, 2026

Hi, I’m Jit, a friendly security platform designed to help developers build secure applications from day zero with an MVS (Minimal viable security) mindset.

In case there are security findings, they will be communicated to you as a comment inside the PR.

Hope you’ll enjoy using Jit.

Questions? Comments? Want to learn more? Get in touch with us.

@olde-ducke olde-ducke marked this pull request as ready for review January 14, 2026 03:39
@ndyakov
Copy link
Copy Markdown
Member

ndyakov commented Jan 14, 2026

Hello @olde-ducke, let's continue the discussion in the issue and we can work on merging a fix.

@olde-ducke olde-ducke marked this pull request as draft January 14, 2026 19:26
…ernals

* Client: return options copy

* ClusterClient: add missing nil options check, copy passed options, return
  options copy

* Ring: copy passed options, return options copy

* SentinelClient: copy passed options
timeout tests now use reflect and unsafe to access clients internal
options
ndyakov
ndyakov previously approved these changes Jan 24, 2026
Copy link
Copy Markdown
Member

@ndyakov ndyakov left a comment

Choose a reason for hiding this comment

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

Hey @olde-ducke, overall i think this is fine. We cannot however make sure that Options is readonly only with this cloning, since for example the PushNotificationProcessor will be the same one and users can Unregister handlers.

After all I am not concerned with problems resulting from such usages of the Options return value. It is since it is documented that the return from Options should be read only, maybe we can add that "Any alteration of the returned *redis.Options may result in undefined behaviour.". What do you think?

Comment thread redis.go
@olde-ducke
Copy link
Copy Markdown
Contributor Author

Yeah, the changes I made do not defend against every possible case. So I think adding note to the doc is better.

I also kept the additional nil check in NewClusterClient from previous commits, since it is the only client that does not check whether options are nil. I hope this is okay.

@olde-ducke olde-ducke marked this pull request as ready for review January 25, 2026 22:02
Comment thread redis.go Outdated
Comment thread osscluster.go
@ndyakov ndyakov merged commit 4427a37 into redis:master Mar 23, 2026
56 of 57 checks passed
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.

invalid memory address or nil pointer dereference in *baseClient.initConn

2 participants