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

Prevent no unique connection bridge name #446

Closed
Tifaifai opened this issue May 12, 2017 · 2 comments
Closed

Prevent no unique connection bridge name #446

Tifaifai opened this issue May 12, 2017 · 2 comments

Comments

@Tifaifai
Copy link
Contributor

Tifaifai commented May 12, 2017

Hello,

In case of no unique connection bridge name in mosquitto.conf, mosquitto crash with a segmentation fault.

example:

connection test
address localhost:1884
topic # both testbrod1/ testbrod1/

connection test
address localhost:1884
topic # both testbrod2/ testbrod2/

We know that it's not a good practice (same name for connection) but the reaction of mosquitto can be better.
A first bad solution is prevent this bug with rename the bridge by default
in conf.c, line 928

cur_bridge = &(config->bridges[config->bridge_count-1]);
memset(cur_bridge, 0, sizeof(struct _mqtt3_bridge));
cur_bridge->name = _mosquitto_strdup(token);
if(!cur_bridge->name){
	_mosquitto_log_printf(NULL, MOSQ_LOG_ERR, "Error: Out of memory.");
	return MOSQ_ERR_NOMEM;
}

replace by:

cur_bridge = &(config->bridges[config->bridge_count-1]);
memset(cur_bridge, 0, sizeof(struct _mqtt3_bridge));

sprintf(token, "%s%d", token, config->bridge_count);

cur_bridge->name = _mosquitto_strdup(token);
if(!cur_bridge->name){
	_mosquitto_log_printf(NULL, MOSQ_LOG_ERR, "Error: Out of memory.");
	return MOSQ_ERR_NOMEM;
}

Have you a better solution for prevent segmentation fault and crash ? because we don't understand this problem (no use bridges[i]->name after).

THX

@Tifaifai
Copy link
Contributor Author

Hello,
We can propose you a new better solution :)
In src/bridge.cin function int mqtt3_bridge_new
Replace:

if(!bridge->remote_clientid){
	if(!gethostname(hostname, 256)){
		len = strlen(hostname) + strlen(bridge->name) + 2;
		id = _mosquitto_malloc(len);
		if(!id){
			return MOSQ_ERR_NOMEM;
		}
		snprintf(id, len, "%s.%s", hostname, bridge->name);
	}else{
		return 1;
	}
bridge->remote_clientid = id;

by:

if(!bridge->remote_clientid){
	if(!gethostname(hostname, 256)){
		len = strlen(hostname) + strlen(bridge->name) + 2 + 1 + snprintf(NULL, 0, "%d", db->bridge_count+1);
		id = _mosquitto_malloc(len);
		if(!id){
			return MOSQ_ERR_NOMEM;
		}
		snprintf(id, len, "%s.%s.%d", hostname, bridge->name, db->bridge_count+1);
	}else{
		return 1;
	}
bridge->remote_clientid = id;

What do you think about it ? Correction ;-) ?

C-U

ralight added a commit that referenced this issue May 12, 2017
Thanks to Tifaifai Maupiti.

Bug: #446
@ralight
Copy link
Contributor

ralight commented May 12, 2017

Thanks for reporting this, I see the crash as well. I've fixed it in a different way by not allowing duplicates.

@ralight ralight closed this as completed May 12, 2017
Tifaifai pushed a commit to Tifaifai/mosquitto that referenced this issue May 15, 2017
Tifaifai pushed a commit to Tifaifai/mosquitto that referenced this issue May 15, 2017
@lock lock bot locked as resolved and limited conversation to collaborators Aug 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants