Re: [PATCH 5/8] mgcp: NUL-terminate MGCP message

On Fri, Nov 29, 2013 at 01:43:47PM +0100, Jacob Erlbeck wrote:

The MGCP message isn't always NUL-terminated when arriving at mgcp_handle_message(). This may lead to undefined results.

oh!

+ /* Ensure that the msg->l2h is NULL terminated. */ + if (msgb_tailroom(msg) > 0) + *msg->tail = '\0'; + else if (*(msg->tail-1) == '\r' || *(msg->tail-1) == '\n') + *(msg->tail - 1) = '\0'; + else { + LOGP(DMGCP, LOGL_ERROR, "Cannot NUL terminate MGCP message: " + "Length: %d, Buffer size: %d\n", + msgb_l2len(msg), msg->data_len); + return NULL; + }

The check misses if "tail - 1" is already \0 and if tail - 1 is not NULL. I would just add an OSMO_ASSERT and fix the caller that didn't null terminate?! What do you think?