Bug 267884
| Summary: | kadmind can read beyond the end of an incoming message's buffer | ||
|---|---|---|---|
| Product: | Base System | Reporter: | Robert Morris <rtm> |
| Component: | bin | Assignee: | Cy Schubert <cy> |
| Status: | Closed FIXED | ||
| Severity: | Affects Some People | CC: | cy, emaste, hps, philip |
| Priority: | --- | ||
| Version: | CURRENT | ||
| Hardware: | Any | ||
| OS: | Any | ||
| URL: | https://reviews.freebsd.org/D37471 | ||
| Attachments: | |||
|
Description
Robert Morris
2022-11-20 20:50:58 UTC
It appears that upstream Heimdal 7.8.0 suffers the same bug. The simplest solution would be to strlcpy() the string to a temporary bufer in match_appl_version(), do the sscanf(), and discard the temporary buffer. I think this should also be reported to our friends at Heimdal. Created attachment 238215 [details]
Ensure terminating NUL is in the string
Making sure that only sscanf() receives a terminating NUL fails to consider that other consumers of her_appl_version (kf) will also encounter the same problem.
This patch ensures that there is always a terminating NUL.
Additionally, if bytes read is zero the version string is not processed. (Though it could be argued that this could be a second commit.)
It appears that upstream Heimdal 7.8.0 has the same problem.
Adding hps@, MAINTAINER of security/heimdal port. Created attachment 238233 [details]
Same patch for both Heimdal ports
The same patch applied to both heimdal ports.
Created attachment 238240 [details]
Ensure terminating NUL is in the string
This patch fixes a -Wparentheses error in the previous version.
Created attachment 238241 [details]
Same patch for both Heimdal ports.
This patch fixes a -Wparentheses error in the previous version.
Patch results: bob# valgrind /usr/libexec/kadmind --debug ==87610== Memcheck, a memory error detector ==87610== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al. ==87610== Using Valgrind-3.20.0 and LibVEX; rerun with -h for copyright info ==87610== Command: /usr/libexec/kadmind --debug ==87610== ==87610== ==87610== HEAP SUMMARY: ==87610== in use at exit: 11,344 bytes in 159 blocks ==87610== total heap usage: 1,052 allocs, 893 frees, 273,855 bytes allocated ==87610== ==87610== LEAK SUMMARY: ==87610== definitely lost: 0 bytes in 0 blocks ==87610== indirectly lost: 0 bytes in 0 blocks ==87610== possibly lost: 437 bytes in 6 blocks ==87610== still reachable: 10,907 bytes in 153 blocks ==87610== suppressed: 0 bytes in 0 blocks ==87610== Rerun with --leak-check=full to see details of leaked memory ==87610== ==87610== For lists of detected and suppressed errors, rerun with: -s ==87610== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 6 from 1) bob# Review posted to https://reviews.freebsd.org/D37471. A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=d7e8666ffb9967a92709a2d2ded4d31568ab1473 commit d7e8666ffb9967a92709a2d2ded4d31568ab1473 Author: Cy Schubert <cy@FreeBSD.org> AuthorDate: 2022-11-21 15:33:08 +0000 Commit: Cy Schubert <cy@FreeBSD.org> CommitDate: 2022-11-24 17:21:13 +0000 heimdal: The version string must always contain a terminating NUL Should the sender send a string without a terminating NUL, ensure that the NUL terminates the string regardless. And while at it only process the version string when bytes are returned. PR: 267884 Reported by: Robert Morris <rtm@lcs.mit.edu> MFC after: 3 days Differential Revision: https://reviews.freebsd.org/D37471 crypto/heimdal/lib/krb5/recvauth.c | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/ports/commit/?id=d831a2fe480fe02126bd5b9aba5569c5e69f1034 commit d831a2fe480fe02126bd5b9aba5569c5e69f1034 Author: Cy Schubert <cy@FreeBSD.org> AuthorDate: 2022-11-21 22:41:13 +0000 Commit: Cy Schubert <cy@FreeBSD.org> CommitDate: 2022-11-24 17:22:01 +0000 security/heimdal*: The version string must always contain a terminating NUL Should the sender send a string without a terminating NUL, ensure that the NUL terminates the string regardless. And while at it only process the version string when bytes are returned. PR: 267884 Reported by: Robert Morris <rtm@lcs.mit.edu> MFH: 2022Q4 security/heimdal-devel/Makefile | 2 +- .../files/patch-lib_krb5_recvauth.c (new) | 42 ++++++++++++++++++++++ security/heimdal/Makefile | 2 +- .../heimdal/files/patch-lib_krb5_recvauth.c (new) | 42 ++++++++++++++++++++++ 4 files changed, 86 insertions(+), 2 deletions(-) A commit in branch stable/13 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=e7dcb18d51088654948840a66ad3864a66f1cb96 commit e7dcb18d51088654948840a66ad3864a66f1cb96 Author: Cy Schubert <cy@FreeBSD.org> AuthorDate: 2022-11-21 15:33:08 +0000 Commit: Cy Schubert <cy@FreeBSD.org> CommitDate: 2022-12-01 14:25:10 +0000 heimdal: The version string must always contain a terminating NUL Should the sender send a string without a terminating NUL, ensure that the NUL terminates the string regardless. And while at it only process the version string when bytes are returned. PR: 267884 Reported by: Robert Morris <rtm@lcs.mit.edu> Differential Revision: https://reviews.freebsd.org/D37471 (cherry picked from commit d7e8666ffb9967a92709a2d2ded4d31568ab1473) crypto/heimdal/lib/krb5/recvauth.c | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) A commit in branch stable/12 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=8d3c6e82cdcac5fa31836df6d3d067efd2e0e3bc commit 8d3c6e82cdcac5fa31836df6d3d067efd2e0e3bc Author: Cy Schubert <cy@FreeBSD.org> AuthorDate: 2022-11-21 15:33:08 +0000 Commit: Cy Schubert <cy@FreeBSD.org> CommitDate: 2022-12-01 14:25:52 +0000 heimdal: The version string must always contain a terminating NUL Should the sender send a string without a terminating NUL, ensure that the NUL terminates the string regardless. And while at it only process the version string when bytes are returned. PR: 267884 Reported by: Robert Morris <rtm@lcs.mit.edu> Differential Revision: https://reviews.freebsd.org/D37471 (cherry picked from commit d7e8666ffb9967a92709a2d2ded4d31568ab1473) crypto/heimdal/lib/krb5/recvauth.c | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) fixed A commit in branch 2022Q4 references this bug: URL: https://cgit.FreeBSD.org/ports/commit/?id=a8c5dca736cdaed68738c39c52a3e0e158951b28 commit a8c5dca736cdaed68738c39c52a3e0e158951b28 Author: Cy Schubert <cy@FreeBSD.org> AuthorDate: 2022-11-21 22:41:13 +0000 Commit: Cy Schubert <cy@FreeBSD.org> CommitDate: 2022-12-05 20:06:14 +0000 security/heimdal*: The version string must always contain a terminating NUL Should the sender send a string without a terminating NUL, ensure that the NUL terminates the string regardless. And while at it only process the version string when bytes are returned. PR: 267884 Reported by: Robert Morris <rtm@lcs.mit.edu> (cherry picked from commit d831a2fe480fe02126bd5b9aba5569c5e69f1034) security/heimdal-devel/Makefile | 2 +- .../files/patch-lib_krb5_recvauth.c (new) | 42 ++++++++++++++++++++++ security/heimdal/Makefile | 2 +- .../heimdal/files/patch-lib_krb5_recvauth.c (new) | 42 ++++++++++++++++++++++ 4 files changed, 86 insertions(+), 2 deletions(-) |
