Jiri Danek created PROTON-1573: ---------------------------------- Summary: Undefined behavior in some calls to memmove in Proton Key: PROTON-1573 URL: https://issues.apache.org/jira/browse/PROTON-1573 Project: Qpid Proton Issue Type: Bug Components: proton-c Affects Versions: proton-c-0.18.0 Reporter: Jiri Danek
Proton sometimes calls to memmove with arguments memmove(?, null, 0), that is, the source pointer is null and length is zero. According to UndefinedBehaviorSanitizer tool (UBSan), this has undefined behavior. {noformat} SUMMARY: AddressSanitizer: undefined-behavior /home/jdanek/Work/repos/qpid-proton/proton-c/src/core/codec.c:268:35 in /home/jdanek/Work/repos/qpid-proton/proton-c/src/core/framing.c:97:39: runtime error: null pointer passed as argument 2, which is declared to never be null {noformat} It can be "fixed" in the following manner, but I think it is probably not worth it. I verified the fix by running UBSan again. Nothing was reported this time. {noformat} diff --git a/proton-c/src/core/buffer.c b/proton-c/src/core/buffer.c index c3015f49..f47acd6d 100644 --- a/proton-c/src/core/buffer.c +++ b/proton-c/src/core/buffer.c @@ -170,8 +170,12 @@ int pn_buffer_append(pn_buffer_t *buf, const char *bytes, size_t size) size_t tail_space = pni_buffer_tail_space(buf); size_t n = pn_min(tail_space, size); + if (n > 0) { memmove(buf->bytes + tail, bytes, n); + } + if (size - n > 0) { memmove(buf->bytes, bytes + n, size - n); + } buf->size += size; diff --git a/proton-c/src/core/framing.c b/proton-c/src/core/framing.c index 09bf4bba..d2c355f8 100644 --- a/proton-c/src/core/framing.c +++ b/proton-c/src/core/framing.c @@ -94,7 +94,9 @@ size_t pn_write_frame(char *bytes, size_t available, pn_frame_t frame) bytes[5] = frame.type; pn_i_write16(&bytes[6], frame.channel); - memmove(bytes + AMQP_HEADER_SIZE, frame.extended, frame.ex_size); + if (frame.ex_size > 0) { + memmove(bytes + AMQP_HEADER_SIZE, frame.extended, frame.ex_size); + } memmove(bytes + 4*doff, frame.payload, frame.size); return size; } else { {noformat} After brief Googling, I believe that although this really is an undefined behavior, it is reasonable to expect that any real-world implementation of memmove will be a simple noop as long as n = 0, which it is always the case. (https://stackoverflow.com/a/5243068/1047788) Probably best to ignore this. -- This message was sent by Atlassian JIRA (v6.4.14#64029) --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org