The allocation did not account for either the trailing null byte or the
space, leading to a buffer overwrite.
This bug was detected by an assertion failure in the allocator. But can
also be easily detected using valgrind:
==25827== Invalid write of size 1
==25827== at 0x6529759: __vsprintf_chk (vsprintf_chk.c:84)
==25827== by 0x65296AC: __sprintf_chk (sprintf_chk.c:31)
==25827== by 0x4D6AB7: sprintf (stdio2.h:33)
==25827== by 0x4D6AB7: proxy_parse_smtpchk_opt (check.c:1799)
==25827== by 0x4A7DDD: cfg_parse_listen (cfgparse-listen.c:2269)
==25827== by 0x494AD3: readcfgfile (cfgparse.c:2167)
==25827== by 0x542995: init (haproxy.c:2021)
==25827== by 0x421DD2: main (haproxy.c:3121)
==25827== Address 0x78712a8 is 0 bytes after a block of size 24 alloc'd
==25827== at 0x4C2FB55: calloc (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==25827== by 0x4D6A8C: proxy_parse_smtpchk_opt (check.c:1797)
==25827== by 0x4A7DDD: cfg_parse_listen (cfgparse-listen.c:2269)
==25827== by 0x494AD3: readcfgfile (cfgparse.c:2167)
==25827== by 0x542995: init (haproxy.c:2021)
==25827== by 0x421DD2: main (haproxy.c:3121)
This patch fixes issue #681.
This bug was introduced in commit fbcc77c6baa7edee85be9c2384d12c55ef651a5a,
which first appeared in 2.2-dev7. No backport needed.
---
src/check.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/check.c b/src/check.c
index 82f18e3a0..c9802a603 100644
--- a/src/check.c
+++ b/src/check.c
@@ -1794,7 +1794,8 @@ int proxy_parse_smtpchk_opt(char **args, int cur_arg,
struct proxy *curpx, struc
cur_arg += 2;
if (*args[cur_arg] && *args[cur_arg+1] &&
(strcmp(args[cur_arg], "EHLO") == 0 || strcmp(args[cur_arg],
"HELO") == 0)) {
- cmd = calloc(strlen(args[cur_arg]) + strlen(args[cur_arg+1]) +
1, sizeof(*cmd));
+ /* <EHLO|HELO> + space (1) + <host> + null byte (1) */
+ cmd = calloc(strlen(args[cur_arg]) + 1 +
strlen(args[cur_arg+1]) + 1, sizeof(*cmd));
if (cmd)
sprintf(cmd, "%s %s", args[cur_arg], args[cur_arg+1]);
}
--
2.27.0
