Uninitialized variable in util_mosq.c

[markhermelinggt]

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.

[ralight]

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]

Roger, Yes, you are right, I had not considered where the third parameter is NULL. Yes, your solution is I think more elegant. I re-ran static analysis with this in place and the problem is gone. I’ll close the bug. Thanks! Mark — Mark Hermeling | GrammaTech | Senior Director Product Marketing mobile +1 (607) 351-5719 | www.grammatech.com

…

On Dec 21, 2017, at 03:38, Roger Light ***@***.***> wrote: 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? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#654 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AY3D0jHwtb1NqZaOTd0AZZyAG3ePc7h9ks5tChj3gaJpZM4RCYmR>.

[markhermelinggt]

This fixes it. Thanks!