This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] Fix invalid memory access when parsing netgroup files with blank lines (BZ #16506)
- From: OndÅej BÃlka <neleai at seznam dot cz>
- To: Siddhesh Poyarekar <siddhesh at redhat dot com>
- Cc: libc-alpha at sourceware dot org
- Date: Mon, 27 Jan 2014 11:27:55 +0100
- Subject: Re: [PATCH] Fix invalid memory access when parsing netgroup files with blank lines (BZ #16506)
- Authentication-results: sourceware.org; auth=none
- References: <20140127070504 dot GD2149 at spoyarek dot pnq dot redhat dot com>
On Mon, Jan 27, 2014 at 12:35:05PM +0530, Siddhesh Poyarekar wrote:
> Hi,
>
> The netgroups file parsing code tries to access the character before
> the newline in parsed lines to see if it is a backslash (\). This
> results in an access before the block allocated for the line if the
> line is blank, i.e. does not have anything other than the newline
> character. This doesn't seem like it will cause any crashes because
> the byte belongs to the malloc metadata block and hence access to it
> will always succeed.
>
> There could be an invalid alteration in code flow where a blank line
> is seen as a continuation due to the preceding byte *happening* to be
> '\\'. This could be done by interposing malloc, but that's not really
> a security problem since one could interpose getnetgrent_r itself and
> achieve a similar 'exploit'.
>
> The possibility of actually exploiting this is remote to impossible
> since it also requires the previous line to end with a '\\', which
> would happen only on invalid configurations.
>
> Verified on x86_64 that this patch fixes the valgrind warning that
> caused me to notice the problem and also does not break netgroups
> queries. OK to commit?
>
> Siddhesh
>
> [BZ #16506]
> * nss/nss_files/files-netgrp.c (_nss_files_setnetgrent): Avoid
> access beyond array bounds when parsing netgroups file.
>
> diff --git a/nss/nss_files/files-netgrp.c b/nss/nss_files/files-netgrp.c
> index 339f704..34eae4c 100644
> --- a/nss/nss_files/files-netgrp.c
> +++ b/nss/nss_files/files-netgrp.c
> @@ -103,7 +103,8 @@ _nss_files_setnetgrent (const char *group, struct __netgrent *result)
> result->cursor += (curlen - group_len) - 1;
> }
>
> - while (line[curlen - 1] == '\n' && line[curlen - 2] == '\\')
> + while (curlen > 1 && line[curlen - 1] == '\n'
> + && line[curlen - 2] == '\\')
> {
> /* Yes, we have a continuation line. */
> if (found)
looks ok.