git: 410556f1f10f - main - libctf: Fix an out-of-bounds read in ctf_lookup_by_name()
Jessica Clarke
jrtc27 at freebsd.org
Sat Mar 27 18:16:47 UTC 2021
On 27 Mar 2021, at 18:06, Mark Johnston <markj at FreeBSD.org> wrote:
>
> The branch main has been updated by markj:
>
> URL: https://cgit.FreeBSD.org/src/commit/?id=410556f1f10fd35b350102725fd8504c3cb0afc8
>
> commit 410556f1f10fd35b350102725fd8504c3cb0afc8
> Author: Domagoj Stolfa <domagoj.stolfa at gmail.com>
> AuthorDate: 2021-03-27 18:04:12 +0000
> Commit: Mark Johnston <markj at FreeBSD.org>
> CommitDate: 2021-03-27 18:04:12 +0000
>
> libctf: Fix an out-of-bounds read in ctf_lookup_by_name()
>
> When prefixes such as struct, union, etc. are compared with the current
> type (e.g. struct foo), a comparison is made with the prefix. The code
> currently assumes that every type is a valid C type with a prefix,
> however at times, garbage ends up in this function causing an
> unpredictable crash with DTrace due to the isspace(*p) call or
> subsequent calls. An example that I've seen of this is the letter 's'
> being passed in, comparing true with struct as the comparison size was
> (q - p) == 1, but then we increment p with the length of "struct",
> resulting in an out of bounds read.
>
> Reviewed by: markj
> MFC after: 1 week
> Differential Revision: https://reviews.freebsd.org/D29435
> ---
> cddl/contrib/opensolaris/common/ctf/ctf_lookup.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/cddl/contrib/opensolaris/common/ctf/ctf_lookup.c b/q
> index aa58663309b6..5912cc1a36e8 100644
> --- a/cddl/contrib/opensolaris/common/ctf/ctf_lookup.c
> +++ b/cddl/contrib/opensolaris/common/ctf/ctf_lookup.c
> @@ -132,8 +132,9 @@ ctf_lookup_by_name(ctf_file_t *fp, const char *name)
> continue; /* skip qualifier keyword */
>
> for (lp = fp->ctf_lookups; lp->ctl_prefix != NULL; lp++) {
> - if (lp->ctl_prefix[0] == '\0' ||
> - strncmp(p, lp->ctl_prefix, (size_t)(q - p)) == 0) {
> + if ((size_t)(q - p) >= lp->ctl_len &&
> + (lp->ctl_prefix[0] == '\0' ||
> + strncmp(p, lp->ctl_prefix, (size_t)(q - p)) == 0)) {
> for (p += lp->ctl_len; isspace(*p); p++)
> continue; /* skip prefix and next ws */
We had a student porting DTrace to CheriBSD as a Master's project notice this
and get this fixed “upstream” in Illumos[1], but neglected to do so in FreeBSD
(and it seems CheriBSD has an earlier version of the patch that I requested be
changed in the upstream review...); you might wish to pull that in instead?
It’s equivalent, just differently formatted, so adds noise to the diff.
Jess
[1] https://github.com/illumos/illumos-gate/commit/d15d17d4231f87f1571fa6d585377206f360f667
More information about the dev-commits-src-main
mailing list