Fix possible null deref in pub - #848
Conversation
| int totalLen = 0; | ||
|
|
||
| if (nc == NULL) | ||
| if (nc == NULL || msg == NULL) |
There was a problem hiding this comment.
No, because normally the caller ensures that it is not NULL. The only exception was natsConnection_PublishMsg() that needs fixing.
The rationale is that this is an internal function and if the caller guarantees that the message is not NULL, then we should not check in this one, which makes it redundant. In lots of places, the caller uses a stack variable and it will never be NULL.
| natsStatus | ||
| natsConnection_PublishMsg(natsConnection *nc, natsMsg *msg) | ||
| { | ||
| const char *reply = (msg != NULL ? msg->reply : NULL); |
There was a problem hiding this comment.
We need to rewrite this function and if msg == NULL return an invalid arg. The check of nc is done in the lower function, but could easily be added into this one.
As for removing the reply subject, of course we need it, but yes, the called function will take it from the provided message structure if not provided as a parameter, so I guess this change is ok.
There was a problem hiding this comment.
@kozlovic "rewrite this function" - you mean add the check/return before msg is even used and remove any conditionals like this?
There was a problem hiding this comment.
No, I meant what the contributor did, that is, check for msg == NULL as an invalid arg and return if that is the case. He did add the check for nc == NULL (although that is done in the internal function so that was not entirely necessary).
9e5d265 to
d3095e4
Compare
d3095e4 to
e490568
Compare
3 participants
remove reply construct because it is done by natsConn_publish
nats.c/src/pub.c
Line 82 in 4f993dc
we have check msg != NULL and i just move it to natsConn_publish
nats.c/src/pub.c
Line 282 in 4f993dc