From 366f0af26d955c27411fd2f1ec906640e937bf74 Mon Sep 17 00:00:00 2001 From: Gary Lockyer Date: Wed, 27 May 2026 11:00:30 +1200 Subject: [PATCH 1/9] lib:util add pointer overflow checks The wrapping of pointer arithmetic is undefined behaviour. Clang from version 20 onwards will treat checks like: ptr + offset < ptr As always evaluating to true. This commit adds the macros: offset_outside_range ptr_overflow BUG: https://bugzilla.samba.org/show_bug.cgi?id=16092 Signed-off-by: Gary Lockyer Reviewed-by: Stefan Metzmacher Reviewed-by: Volker Lendecke (Backported from commit 2e53f7196f45d28689f25a57fa84995eceee4585) test_json_logging not present in v23 --- lib/util/overflow.h | 57 +++++++++++++++++++++++ lib/util/tests/test_overflow.c | 82 ++++++++++++++++++++++++++++++++++ lib/util/wscript_build | 6 +++ 3 files changed, 145 insertions(+) create mode 100644 lib/util/overflow.h create mode 100644 lib/util/tests/test_overflow.c diff --git a/lib/util/overflow.h b/lib/util/overflow.h new file mode 100644 index 00000000000..ec9ac179e19 --- /dev/null +++ b/lib/util/overflow.h @@ -0,0 +1,57 @@ +/* + * Unix SMB/CIFS implementation. + * Samba utility functions + * + * Copyright (C) Gary Lockyer 2026 + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#ifndef __LIB_UTIL_OVERFLOW_H__ +#define __LIB_UTIL_OVERFLOW_H__ + +#include +/** +* Will adding offset to base pointer, result in a pointer that points +* past end, or overflow +* +* @param base The pointer to the start of the range +* @param end The pointer to the end of the range +* @param offset The offset being added to base +* +* @return True resulting pointer is between base and end +* False resulting pointer is after end +*/ +#define offset_outside_range(base, end, offset) \ + (((end) - (base)) < (offset)) + +/** +* Will adding offset to base pointer, result in overflow. +* Pointer arithmetic overflow is undefined behaviour and some compilers +* (i.e. Clang from version 20) will treat an overflow check like the following: +* (ptr + offset) < ptr +* as always evaluating to false +* +* @param ptr The pointer to check +* @param offset The offset being added to ptr +* @param type Type being pointed to by pointer +* needed to cast INTPTR_MAX to the correct type +* +* @return True pointer would over flow +* False pointer would not overflow +*/ +#define ptr_overflow(ptr, offset, type) \ + offset_outside_range((ptr), ((type*)INTPTR_MAX), (offset)) + +#endif diff --git a/lib/util/tests/test_overflow.c b/lib/util/tests/test_overflow.c new file mode 100644 index 00000000000..437374dd9dc --- /dev/null +++ b/lib/util/tests/test_overflow.c @@ -0,0 +1,82 @@ +/* + * cmocka unit tests for the overflow macros + * + * Copyright (C) Gary Lockyer 2026 + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * + */ + +/* + * from cmocka.c: + * These headers or their equivalents should be included prior to + * including + * this header file. + * + * #include + * #include + * #include + * + * This allows test applications to use custom definitions of C standard + * library functions and types. + * + */ +#include +#include +#include +#include + + +#include "lib/util/overflow.h" + +static void test_ptr_overflow_true(void **state) +{ + char *ptr = (char *)INTPTR_MAX; + assert_true(ptr_overflow(ptr, 1, char)); +} + +static void test_ptr_overflow_false(void **state) +{ + char *ptr = (char *)(INTPTR_MAX- 1); + assert_false(ptr_overflow(ptr, 1, char)); +} + +static void test_outside_range_false(void **state) +{ + char base[] = "1234"; + char *end = base + 5; + assert_false(offset_outside_range(base, end, 5)); +} + +static void test_outside_range_true(void **state) +{ + char base[] = "1234"; + char *end = base + 5; + assert_true(offset_outside_range(base, end, 6)); +} + +int main(int argc, const char **argv) +{ + const struct CMUnitTest tests[] = { + cmocka_unit_test( test_ptr_overflow_true), + cmocka_unit_test( test_ptr_overflow_false), + cmocka_unit_test( test_outside_range_false), + cmocka_unit_test( test_outside_range_true), + }; + + cmocka_set_message_output(CM_OUTPUT_SUBUNIT); + + return cmocka_run_group_tests(tests, NULL, NULL); + +} diff --git a/lib/util/wscript_build b/lib/util/wscript_build index c9c04f1aaed..57b2788471e 100644 --- a/lib/util/wscript_build +++ b/lib/util/wscript_build @@ -398,6 +398,12 @@ else: local_include=False, for_selftest=True) + bld.SAMBA_BINARY('test_overflow', + source='tests/test_overflow.c', + deps='cmocka', + local_include=False, + for_selftest=True) + bld.SAMBA_BINARY('test_s4_logging', source='tests/test_logging.c', deps=' '.join(['CMDLINE_S4', -- 2.43.0 From 1b668341bb1a158c9a349d5ff6c96b0c1015bd0a Mon Sep 17 00:00:00 2001 From: Gary Lockyer Date: Wed, 15 Apr 2026 10:58:15 +1200 Subject: [PATCH 2/9] s3:libsmb:clilist fix tautological-compare The wrapping of pointer arithmetic is undefined behaviour. Clang from version 20 onwards will treat an overflow check of the following form: ptr + offset < ptr as always evaluating to false. BUG: https://bugzilla.samba.org/show_bug.cgi?id=16092 Signed-off-by: Gary Lockyer Reviewed-by: Stefan Metzmacher Reviewed-by: Volker Lendecke --- source3/libsmb/clilist.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/source3/libsmb/clilist.c b/source3/libsmb/clilist.c index 9ec3e3240f4..f8d23c634cc 100644 --- a/source3/libsmb/clilist.c +++ b/source3/libsmb/clilist.c @@ -22,6 +22,7 @@ #include "source3/libsmb/proto.h" #include "source3/libsmb/cli_smb2_fnum.h" #include "../lib/util/tevent_ntstatus.h" +#include "../lib/util/overflow.h" #include "async_smb.h" #include "trans2.h" #include "../libcli/smb/smbXcli_base.h" @@ -97,8 +98,8 @@ static size_t calc_next_entry_offset(const char *base, const char *pdata_end) size_t next_entry_offset = (size_t)IVAL(base,0); if (next_entry_offset == 0 || - base + next_entry_offset < base || - base + next_entry_offset > pdata_end) { + offset_outside_range(base, pdata_end, next_entry_offset)) + { next_entry_offset = pdata_end - base; } return next_entry_offset; @@ -283,7 +284,7 @@ static size_t interpret_long_filename(TALLOC_CTX *ctx, return pdata_end - base; } p += 24; /* short name? */ - if (p + namelen < p || p + namelen > pdata_end) { + if (offset_outside_range(p, pdata_end, namelen)) { return pdata_end - base; } ret = pull_string_talloc(ctx, -- 2.43.0 From d3f9b33d63812dee321243a9388c96b5918aad32 Mon Sep 17 00:00:00 2001 From: Gary Lockyer Date: Wed, 15 Apr 2026 11:11:35 +1200 Subject: [PATCH 3/9] libcli:auth:msrpc_parse fix white space BUG: https://bugzilla.samba.org/show_bug.cgi?id=16092 Signed-off-by: Gary Lockyer Reviewed-by: Stefan Metzmacher Reviewed-by: Volker Lendecke --- libcli/auth/msrpc_parse.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/libcli/auth/msrpc_parse.c b/libcli/auth/msrpc_parse.c index bdabcaff735..f8cbeb4df9c 100644 --- a/libcli/auth/msrpc_parse.c +++ b/libcli/auth/msrpc_parse.c @@ -1,20 +1,20 @@ -/* +/* Unix SMB/CIFS implementation. simple kerberos5/SPNEGO routines Copyright (C) Andrew Tridgell 2001 Copyright (C) Jim McDonough 2002 Copyright (C) Andrew Bartlett 2002-2003 - + This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation; either version 3 of the License, or (at your option) any later version. - + This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details. - + You should have received a copy of the GNU General Public License along with this program. If not, see . */ @@ -40,7 +40,7 @@ d = word (4 bytes) C = constant ascii string */ -NTSTATUS msrpc_gen(TALLOC_CTX *mem_ctx, +NTSTATUS msrpc_gen(TALLOC_CTX *mem_ctx, DATA_BLOB *blob, const char *format, ...) { @@ -182,7 +182,7 @@ NTSTATUS msrpc_gen(TALLOC_CTX *mem_ctx, break; case 'd': j = intargs[i]; - SIVAL(blob->data, head_ofs, j); + SIVAL(blob->data, head_ofs, j); head_ofs += 4; break; case 'b': @@ -204,7 +204,7 @@ NTSTATUS msrpc_gen(TALLOC_CTX *mem_ctx, } } va_end(ap); - + talloc_free(pointers); return NT_STATUS_OK; @@ -231,7 +231,7 @@ if ((head_ofs + amount) > blob->length) { \ C = constant ascii string */ -bool msrpc_parse(TALLOC_CTX *mem_ctx, +bool msrpc_parse(TALLOC_CTX *mem_ctx, const DATA_BLOB *blob, const char *format, ...) { @@ -317,8 +317,8 @@ bool msrpc_parse(TALLOC_CTX *mem_ctx, if (0 < len1) { size_t pull_len; - if (!convert_string_talloc(mem_ctx, CH_DOS, CH_UNIX, - blob->data + ptr, len1, + if (!convert_string_talloc(mem_ctx, CH_DOS, CH_UNIX, + blob->data + ptr, len1, ps, &pull_len)) { ret = false; goto cleanup; -- 2.43.0 From 5f3eb8e23e106c5c4dde565683caf068ff2b00cb Mon Sep 17 00:00:00 2001 From: Gary Lockyer Date: Wed, 15 Apr 2026 11:13:16 +1200 Subject: [PATCH 4/9] libcli:auth:msrpc_parse fix tautological-compare The wrapping of pointer arithmetic is undefined behaviour. Clang from version 20 onwards will treat an overflow check of the following form: ptr + offset < ptr as always evaluating to false BUG: https://bugzilla.samba.org/show_bug.cgi?id=16092 Signed-off-by: Gary Lockyer Reviewed-by: Stefan Metzmacher Reviewed-by: Volker Lendecke (cherry picked from commit d80c9dac0765a5b114718450ec5a062cd2b6f86f) --- libcli/auth/msrpc_parse.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/libcli/auth/msrpc_parse.c b/libcli/auth/msrpc_parse.c index f8cbeb4df9c..784621ef369 100644 --- a/libcli/auth/msrpc_parse.c +++ b/libcli/auth/msrpc_parse.c @@ -20,6 +20,7 @@ */ #include "includes.h" +#include "lib/util/overflow.h" #include "libcli/auth/msrpc_parse.h" /* @@ -274,8 +275,7 @@ bool msrpc_parse(TALLOC_CTX *mem_ctx, ret = false; goto cleanup; } - if (blob->data + ptr < (uint8_t *)(uintptr_t)ptr || - blob->data + ptr < blob->data) { + if (ptr_overflow(blob->data, ptr, uint8_t)) { ret = false; goto cleanup; } @@ -308,8 +308,8 @@ bool msrpc_parse(TALLOC_CTX *mem_ctx, goto cleanup; } - if (blob->data + ptr < (uint8_t *)(uintptr_t)ptr || - blob->data + ptr < blob->data) { + if (ptr_overflow(blob->data, ptr, uint8_t)) + { ret = false; goto cleanup; } @@ -348,8 +348,7 @@ bool msrpc_parse(TALLOC_CTX *mem_ctx, goto cleanup; } - if (blob->data + ptr < (uint8_t *)(uintptr_t)ptr || - blob->data + ptr < blob->data) { + if (ptr_overflow(blob->data, ptr, uint8_t)) { ret = false; goto cleanup; } @@ -362,8 +361,7 @@ bool msrpc_parse(TALLOC_CTX *mem_ctx, len1 = va_arg(ap, unsigned int); /* make sure its in the right format - be strict */ NEED_DATA(len1); - if (blob->data + head_ofs < (uint8_t *)head_ofs || - blob->data + head_ofs < blob->data) { + if (ptr_overflow(blob->data, head_ofs, uint8_t)) { ret = false; goto cleanup; } @@ -379,9 +377,9 @@ bool msrpc_parse(TALLOC_CTX *mem_ctx, case 'C': s = va_arg(ap, char *); - if (blob->data + head_ofs < (uint8_t *)head_ofs || - blob->data + head_ofs < blob->data || - (head_ofs + (strlen(s) + 1)) > blob->length) { + if (ptr_overflow(blob->data, head_ofs, uint8_t) || + (head_ofs + (strlen(s) + 1)) > blob->length) + { ret = false; goto cleanup; } -- 2.43.0 From e03448daf269d515fdcf61230bfb6885bce22513 Mon Sep 17 00:00:00 2001 From: Gary Lockyer Date: Wed, 15 Apr 2026 12:33:08 +1200 Subject: [PATCH 5/9] s3/torture/test_smb1_dfs fix tautological-compare The wrapping of pointer arithmetic is undefined behaviour. Clang from version 20 onwards will treat an overflow check of the following form: ptr + offset < ptr as always evaluating to false. BUG: https://bugzilla.samba.org/show_bug.cgi?id=16092 Signed-off-by: Gary Lockyer Reviewed-by: Stefan Metzmacher Reviewed-by: Volker Lendecke (cherry picked from commit 769cf0a0ddb5c7af65afa5b764c24a3416025745) --- source3/torture/test_smb1_dfs.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/source3/torture/test_smb1_dfs.c b/source3/torture/test_smb1_dfs.c index 4cd75c9e056..8ea9cd51fac 100644 --- a/source3/torture/test_smb1_dfs.c +++ b/source3/torture/test_smb1_dfs.c @@ -32,6 +32,7 @@ #include "async_smb.h" #include "../lib/util/tevent_ntstatus.h" #include "lib/util/time_basic.h" +#include "lib/util/overflow.h" extern fstring host, workgroup, share, password, username, myname; extern struct cli_credentials *torture_creds; @@ -1949,8 +1950,8 @@ static size_t calc_next_entry_offset(const uint8_t *base, size_t next_entry_offset = (size_t)PULL_LE_U32(base,0); if (next_entry_offset == 0 || - base + next_entry_offset < base || - base + next_entry_offset > pdata_end) { + offset_outside_range(base, pdata_end, next_entry_offset)) + { next_entry_offset = pdata_end - base; } return next_entry_offset; @@ -2009,7 +2010,7 @@ static size_t get_filename(TALLOC_CTX *ctx, return pdata_end - base; } p += 24; /* short name */ - if (p + namelen < p || p + namelen > pdata_end) { + if (offset_outside_range(p, pdata_end, namelen)) { return pdata_end - base; } ret = pull_string_talloc(ctx, -- 2.43.0 From eff63aaf402b615341f3efdb89418d5d3999ec82 Mon Sep 17 00:00:00 2001 From: Gary Lockyer Date: Wed, 15 Apr 2026 12:35:24 +1200 Subject: [PATCH 6/9] s3:utils:clirap2 fix tautological-compare The wrapping of pointer arithmetic is undefined behaviour. Clang from version 20 onwards will treat an overflow check of the following form: ptr + offset < ptr as always evaluating to false BUG: https://bugzilla.samba.org/show_bug.cgi?id=16092 Signed-off-by: Gary Lockyer Reviewed-by: Stefan Metzmacher Reviewed-by: Volker Lendecke (cherry picked from commit 3287641bbf3360c8241db090781ebc2d5febbc96) --- source3/utils/clirap2.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/source3/utils/clirap2.c b/source3/utils/clirap2.c index 61f46980fff..d8ef486f6bd 100644 --- a/source3/utils/clirap2.c +++ b/source3/utils/clirap2.c @@ -82,6 +82,7 @@ #include "../librpc/gen_ndr/svcctl.h" #include "clirap2.h" #include "../libcli/smb/smbXcli_base.h" +#include "lib/util/overflow.h" #define WORDSIZE 2 #define DWORDSIZE 4 @@ -212,7 +213,7 @@ static size_t rap_getstringp(TALLOC_CTX *ctx, char *p, char **dest, char *r, uin off &= 0x0000FFFF; /* mask the obsolete segment number from the offset */ off -= c; } - if (r+off > endp || r+off < r) { + if (offset_outside_range(r, endp, off)) { src=""; len=1; } else { -- 2.43.0 From f8dc4dcce294c54a93cf3976f4b0d94b844fea11 Mon Sep 17 00:00:00 2001 From: Gary Lockyer Date: Wed, 15 Apr 2026 13:32:11 +1200 Subject: [PATCH 7/9] s3:libsmb:cliquota fix tautological-compare The wrapping of pointer arithmetic is undefined behaviour. Clang from version 20 onwards will treat an overflow check of the following form: ptr + offset < ptr as always evaluating to false BUG: https://bugzilla.samba.org/show_bug.cgi?id=16092 Signed-off-by: Gary Lockyer Reviewed-by: Stefan Metzmacher Reviewed-by: Volker Lendecke (cherry picked from commit b58d7045d5444d70db58d07d7746006ea1b9ecfb) --- source3/libsmb/cliquota.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/source3/libsmb/cliquota.c b/source3/libsmb/cliquota.c index 865a41ff618..40dfae9b13f 100644 --- a/source3/libsmb/cliquota.c +++ b/source3/libsmb/cliquota.c @@ -27,6 +27,7 @@ #include "trans2.h" #include "../libcli/smb/smbXcli_base.h" #include "librpc/gen_ndr/ndr_quota.h" +#include "lib/util/overflow.h" NTSTATUS cli_get_quota_handle(struct cli_state *cli, uint16_t *quota_fnum) { @@ -137,7 +138,7 @@ NTSTATUS parse_user_quota_list(const uint8_t *curdata, break; } - if (curdata + offset < curdata) { + if (ptr_overflow(curdata, offset, uint8_t)) { DEBUG(1, ("Pointer overflow in quota record\n")); status = NT_STATUS_INVALID_NETWORK_RESPONSE; break; -- 2.43.0 From d39ce1180e6cf8babd45ea60a2c4f7e264ada653 Mon Sep 17 00:00:00 2001 From: Gary Lockyer Date: Thu, 28 May 2026 09:39:30 +1200 Subject: [PATCH 8/9] build: enable tautological-compare errors Now that all the warnings have been removed, and as they did in fact reveal real issues lets make it an error. BUG: https://bugzilla.samba.org/show_bug.cgi?id=16092 Signed-off-by: Gary Lockyer Reviewed-by: Stefan Metzmacher Reviewed-by: Volker Lendecke (cherry picked from commit 3485f58ea1c2a4013b0ab60c1ddee0faa4eb3eea) --- buildtools/wafsamba/samba_autoconf.py | 1 - 1 file changed, 1 deletion(-) diff --git a/buildtools/wafsamba/samba_autoconf.py b/buildtools/wafsamba/samba_autoconf.py index 979d5a3972f..7b6b2165513 100644 --- a/buildtools/wafsamba/samba_autoconf.py +++ b/buildtools/wafsamba/samba_autoconf.py @@ -846,7 +846,6 @@ int main(void) { if not Options.options.disable_warnings_as_errors: conf.ADD_NAMED_CFLAGS('PICKY_CFLAGS', '-Werror -Wno-error=deprecated-declarations', testflags=True) - conf.ADD_NAMED_CFLAGS('PICKY_CFLAGS', '-Wno-error=tautological-compare', testflags=True) conf.ADD_NAMED_CFLAGS('PICKY_CFLAGS', '-Wno-error=cast-align', testflags=True) if Options.options.fatal_errors: -- 2.43.0 From 043cfeb60543a4c28202bec48fd98e27084dc3a6 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 28 May 2026 11:26:31 +0200 Subject: [PATCH 9/9] build: add -Werror=tautological-compare BUG: https://bugzilla.samba.org/show_bug.cgi?id=16092 Signed-off-by: Stefan Metzmacher Reviewed-by: Volker Lendecke (Backported from commit fb188140357a5d8796c47c5fd88c8a25117bda8e) -Werror=array-bounds and -Werror=stringop-overlow --- buildtools/wafsamba/samba_autoconf.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/buildtools/wafsamba/samba_autoconf.py b/buildtools/wafsamba/samba_autoconf.py index 7b6b2165513..bf0b9bd52da 100644 --- a/buildtools/wafsamba/samba_autoconf.py +++ b/buildtools/wafsamba/samba_autoconf.py @@ -812,7 +812,8 @@ def SAMBA_CONFIG_H(conf, path=None): testflags=True) conf.ADD_CFLAGS('-Werror=old-style-definition -Wold-style-definition', testflags=True) - + conf.ADD_CFLAGS('-Werror=tautological-compare', + testflags=True) conf.ADD_CFLAGS('-Wformat=2 -Wno-format-y2k', testflags=True) conf.ADD_CFLAGS('-Wno-format-zero-length', testflags=True) conf.ADD_CFLAGS('-Werror=format-security -Wformat-security', -- 2.43.0