[SLOF] [PATCH 2/2] lib/libnet/pxelinux: Fix two off-by-one bugs in the pxelinux.cfg parser

[Greg Kurz]

On Fri,  1 Jun 2018 07:26:51 +0200
Thomas Huth <thuth at redhat.com> wrote:

> There are two small bugs in the pxelinux.cfg parser:
> 
> 1. If the file does not end with a '\n', the code set 'eol = cfg + cfgsize'
> and later wrote a NUL character to *eol, i.e. it wrote the NUL character
> beyond the end of the buffer. We've got to use 'eol = cfg + cfgsize - 1'
> instead.
> 
> 2. The code always replaced the last byte of the buffer with a NUL character
> to get a proper termination. If the config file ends with a required character
> (e.g. the last line is a KERNEL or INITRD line and the file does not have
> a '\n' at the end), the last character got lost. Move the obligation for the
> terminating NUL character to the caller instead so that we can be sure to
> have a proper terminated buffer in pxelinux_parse_cfg() without the need to
> blindly overwrite the last character here.
> 
> Signed-off-by: Thomas Huth <thuth at redhat.com>
> ---

Reviewed-by: Greg Kurz <groug at kaod.org>

>  lib/libnet/pxelinux.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/libnet/pxelinux.c b/lib/libnet/pxelinux.c
> index eaead48..7ef09bc 100644
> --- a/lib/libnet/pxelinux.c
> +++ b/lib/libnet/pxelinux.c
> @@ -150,8 +150,9 @@ static int pxelinux_load_cfg(filename_ip_t *fn_ip, uint8_t *mac, const char *uui
>   * in entries point to the original location in the cfg buffer area. The cfg
>   * buffer is altered for this, too, e.g. terminating NUL-characters are put
>   * into the right locations.
> - * @param cfg          Pointer to the buffer with contents of the config file
> - * @param cfgsize      Size of the cfg buffer
> + * @param cfg          Pointer to the buffer with contents of the config file.
> + *                     The caller must make sure that it is NUL-terminated.
> + * @param cfgsize      Size of the cfg data (including the terminating NUL)
>   * @param entries      Pointer to array where the results should be put into
>   * @param max_entries  Number of available slots in the entries array
>   * @param def_ent      Used to return the index of the default entry
> @@ -166,12 +167,10 @@ int pxelinux_parse_cfg(char *cfg, int cfgsize, struct pl_cfg_entry *entries,
>  
>  	*def_ent = 0;
>  
> -	cfg[cfgsize - 1] = 0;  /* Make sure it is NUL-terminated */
> -
>  	while (ptr < cfg + cfgsize && num_entries < max_entries) {
>  		eol = strchr(ptr, '\n');
>  		if (!eol) {
> -			eol = cfg + cfgsize;
> +			eol = cfg + cfgsize - 1;
>  		}
>  		nextptr = eol + 1;
>  		do {
> @@ -248,5 +247,7 @@ int pxelinux_load_parse_cfg(filename_ip_t *fn_ip, uint8_t *mac, const char *uuid
>  		return -1;
>  	}
>  
> +	cfgbuf[rc++] = '\0';	/* Make sure it is NUL-terminated */
> +
>  	return pxelinux_parse_cfg(cfgbuf, rc, entries, max_entries, def_ent);
>  }