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

Uninitialized variable in util_mosq.c #654

Closed
markhermelinggt opened this Issue Dec 14, 2017 · 3 comments

Comments

Projects
None yet
2 participants
@markhermelinggt

markhermelinggt commented Dec 14, 2017

Line 231 reads:
if(!sub || !topic || !result) return MOSQ_ERR_INVAL;

However, result is passed in as the 3rd parameter of
int mosquitto_topic_matches_sub(const char *sub, const char *topic, bool *result)

and often, it is not initialized before passed in, for example in
security_default.c:256

Instead, I recommend to change this to:
if(!sub || !topic )
{
*result = false;
return MOSQ_ERR_INVAL;
}

I don't have submitting rights, will work on that next, but wanted to log this first.

This was found using GrammaTech CodeSonar.

markhermelinggt added a commit to markhermelinggt/mosquitto that referenced this issue Dec 14, 2017

Fix for eclipse#654
Fix for uninitialized variable

ralight added a commit that referenced this issue Dec 21, 2017

@ralight

This comment has been minimized.

Contributor

ralight commented Dec 21, 2017

Thanks for the report, but your solution isn't quite right. The check that you highlight is to discover whether the pointers passed to the function are valid or not, so crudely the difference between these lines:

mosquitto_topic_matches_sub(sub, topic, &result)
mosquitto_topic_matches_sub(sub, topic, NULL);

If the result pointer is NULL, we can't write into it and there's no way to return the matches result, so we return MOSQ_ERR_INVAL.

I've added a fix that I believe improves the function as much as it can be in this regard, if you agree that it does the job would you close the bug?

@markhermelinggt

This comment has been minimized.

markhermelinggt commented Dec 21, 2017

@markhermelinggt

This comment has been minimized.

markhermelinggt commented Dec 21, 2017

This fixes it. Thanks!

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