Created attachment 243358 [details] fix bcmp() bug bcmp() returns the number of remaining bytes when the main loop exits. In case of a match, this is zero, else a positive integer. On systems where SIZE_MAX > INT_MAX, the implicit conversion from size_t to int in the return value may cause the number of remaining bytes to overflow, becoming zero and falsely indicating a successful comparison. This bug affects any 64 bit system that doesn't have a machdep implementation of bcmp(). This should be all except amd64. OpenBSD has the same implementation and is likely affected, too. The bug has presumably been present since ancient times. The attached patch fixes the bug by always returning 0 on equality, 1 otherwise.
With such a long lived bug, I might be tempted to go with: return (length <= (size_t)INT_MAX ? length : -1);
Note that machdep implementations already do not seem to return the exact same return value as the generic version. Please add code review comments to the DR linked. https://reviews.freebsd.org/D41011
(In reply to Robert Clausecker from comment #0) OpenBSD is not affected: https://marc.info/?l=openbsd-bugs&m=168927619115526&w=2
(In reply to Mikhail Pchelin from comment #3) Indeed, they fixed their buggy copy (in libkern) right after I submitted the bug report: https://github.com/openbsd/src/commit/996ed212dfaa3a04ce46abccc4c9546db1b8a756
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=4da7282a1882fc03c99591c27d44a2e6dfda364b commit 4da7282a1882fc03c99591c27d44a2e6dfda364b Author: Robert Clausecker <fuz@FreeBSD.org> AuthorDate: 2023-07-12 18:23:21 +0000 Commit: Robert Clausecker <fuz@FreeBSD.org> CommitDate: 2023-07-16 17:36:17 +0000 lib/libc/string/bcmp.c: fix integer overflow bug bcmp() returned the number of remaining bytes when the main loop exits. In case of a match, this is zero, else a positive integer. On systems where SIZE_MAX > INT_MAX, the implicit conversion from size_t to int in the return value may cause the number of remaining bytes to overflow, becoming zero and falsely indicating a successful comparison. Fix the bug by always returning 0 on equality, 1 otherwise. PR: 272474 Approved by: emaste Reviewed by: imp MFC After: 1 week Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D41011 lib/libc/string/bcmp.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
A commit in branch stable/13 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=3b0dacf6fd13cf8688402561192899507f46c276 commit 3b0dacf6fd13cf8688402561192899507f46c276 Author: Robert Clausecker <fuz@FreeBSD.org> AuthorDate: 2023-07-12 18:23:21 +0000 Commit: Robert Clausecker <fuz@FreeBSD.org> CommitDate: 2023-07-21 08:54:08 +0000 lib/libc/string/bcmp.c: fix integer overflow bug bcmp() returned the number of remaining bytes when the main loop exits. In case of a match, this is zero, else a positive integer. On systems where SIZE_MAX > INT_MAX, the implicit conversion from size_t to int in the return value may cause the number of remaining bytes to overflow, becoming zero and falsely indicating a successful comparison. Fix the bug by always returning 0 on equality, 1 otherwise. PR: 272474 Approved by: emaste Reviewed by: imp MFC After: 1 week Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D41011 (cherry picked from commit 4da7282a1882fc03c99591c27d44a2e6dfda364b) lib/libc/string/bcmp.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
A commit in branch stable/12 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=99544e13eec1586552470bd9d5f3b24038891401 commit 99544e13eec1586552470bd9d5f3b24038891401 Author: Robert Clausecker <fuz@FreeBSD.org> AuthorDate: 2023-07-12 18:23:21 +0000 Commit: Robert Clausecker <fuz@FreeBSD.org> CommitDate: 2023-07-21 08:57:32 +0000 lib/libc/string/bcmp.c: fix integer overflow bug bcmp() returned the number of remaining bytes when the main loop exits. In case of a match, this is zero, else a positive integer. On systems where SIZE_MAX > INT_MAX, the implicit conversion from size_t to int in the return value may cause the number of remaining bytes to overflow, becoming zero and falsely indicating a successful comparison. Fix the bug by always returning 0 on equality, 1 otherwise. PR: 272474 Approved by: emaste Reviewed by: imp MFC After: 1 week Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D41011 (cherry picked from commit 4da7282a1882fc03c99591c27d44a2e6dfda364b) lib/libc/string/bcmp.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)