Community
Participate
Working Groups
In version 1.6.9 of Mosquitto (https://github.com/eclipse/mosquitto), there is a memory error (out of bound pointer) in net_mosq.c Currently, the line that causes this behavior is (net_mosq.c:401): *sock = socket(rp->ai_family, rp->ai_socktype, rp->ai_protocol); Stack: #000011082 in net__try_connect (=63048704, =1883, =67297944, =0, =true) at net_mosq.c:401 #100010470 in net__socket_connect (=54744320, =63048704, =1883, =0, =true) at net_mosq.c:836 #200010101 in mosquitto__reconnect (=54744320, =true, =0) at connect.c:201 #300011456 in mosquitto_connect_bind_v5 (=54744320, =63049472, =1883, =60, =0, =0) at connect.c:124 #400021660 in client_connect (=54744320, =48519104) at client_shared.c:1201 I found this error through testing on 64-bit Ubuntu, using the Make build system, not CMake.
*sock = socket(rp->ai_family, rp->ai_socktype, rp->ai_protocol); If there is a pointer error on this line, it is either the sock pointer or the rp pointer that is invalid. sock is passed to the function, rp is an internal variable. Looking at rp first, it is assigned as part of the for loop: ``` for(rp = ainfo; rp != NULL; rp = rp->ai_next){ *sock = socket(rp->ai_family, rp->ai_socktype, rp->ai_protocol); ``` rp is set to ainfo, and then the loop runs if rp is not null. If ainfo was set to a non-null value that was not valid then this could cause a problem. ainfo is set earlier in the function with ``` s = getaddrinfo(host, NULL, &hints, &ainfo); if(s){ errno = s; return MOSQ_ERR_EAI; } ``` getaddrinfo will retun 0 if it succeeds, at which point we must assume that ainfo is valid. If getaddrinfo does not succeed, s will be non-zero, and so the function will exit with an error before it gets to the for loop and the problem line. Now looking at the sock pointer, there is no explicit check for validity, so it is feasible that a null pointer is being passed. The arguments against this are that the assignment `*sock = INVALID_SOCKET` is called as the first line of the function, so we would expect it to fail immediately on a null or invalid pointer, rather than later on. From your stack trace it seems as though the sock pointer is not null anyway - it is set to 67297944. The only place where net__try_connect() is called is from net__socket_connect(): ``` mosq_sock_t sock = INVALID_SOCKET; int rc, rc2; if(!mosq || !host || !port) return MOSQ_ERR_INVAL; rc = net__try_connect(host, port, &sock, bind_address, blocking); ``` So sock is always a valid pointer. I don't understand how what you are seeing could be possible. Could you please make sure you are using a clean copy of the source, run `make clean`, then `make` again to ensure that there weren't any old object files from a previous version and check whether the error you are seeing still occurs. If it does, please run the code with `valgrind --log-file=vglog <the program with options>` and attach vglog.
And most importantly, thank you for reporting this responsibly!
Thanks for the detailed writeup! I've tried with a clean copy of the source, but I will investigate a bit further with valgrind to confirm whether or not I still see that behavior.
Housekeeping. We've exceeded the three month deadline for disclosure, so I've removed the confidential flag. I've marked the issue as INVALID based on the discussion. Please feel free to reopen if you feel that I've done so in error.
