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: [email protected]
For additional commands, e-mail: [email protected]
