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

nil pointer dereferencing #1752

Open
MAGeorg opened this issue Dec 17, 2024 · 3 comments
Open

nil pointer dereferencing #1752

MAGeorg opened this issue Dec 17, 2024 · 3 comments
Labels
defect Suspected defect such as a bug or regression

Comments

@MAGeorg
Copy link

MAGeorg commented Dec 17, 2024

Observed behavior

Hello,

We checked our code with SAST tool and found some issues in nats library.

  1. jetstream/jetstream.go:904 On line 904 we can potentially call close() on an uninitialized channel, which will lead to a panic.
  2. nats.go:4649-4651 On lines 4649-4651 sub is checked for nil, if sub is nil, then return is called, before which defer is called, where the nil pointer is dereferenced.

Expected behavior

Added checks for nil

Server and client version

nats go lib version - v1.34.1 (but the bugs are also relevant for the main branch)

Host environment

No response

Steps to reproduce

No response

@MAGeorg MAGeorg added the defect Suspected defect such as a bug or regression label Dec 17, 2024
@piotrpio
Copy link
Collaborator

Hello @MAGeorg, thank you for creating the issue.

Thanks for catching those. The first one should definitely be fixed, I'll take care of that.
The second one I'm not sure I get. The lines you're pointing to are:

// changeSubStatus changes subscription status and sends events
// to all listeners. Lock should be held entering.
func (s *Subscription) changeSubStatus(status SubStatus) {
	if s == nil {
		return
	}
	s.sendStatusEvent(status)
	s.status = status
}

Not sure where the nil pointer dereference, plus there is no defer in this method. Could you please elaborate?

@MAGeorg
Copy link
Author

MAGeorg commented Dec 18, 2024

Hello @piotrpio
Sorry, I probably made a mistake when I attached the link.
In the second problem I was talking about the following code on line nats.go:4687-4695

// checkDrained will watch for a subscription to be fully drained
// and then remove it.
func (nc *Conn) checkDrained(sub *Subscription) {
	defer func() {
		sub.mu.Lock()
		defer sub.mu.Unlock()
		sub.draining = false
	}()
	if nc == nil || sub == nil {
		return
	}

...

@piotrpio
Copy link
Collaborator

piotrpio commented Dec 18, 2024

Thank you! I'll take care of those.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
defect Suspected defect such as a bug or regression
Projects
None yet
Development

No branches or pull requests

2 participants