Bug 211958 - Boot overflows when reading loader.conf
Summary: Boot overflows when reading loader.conf
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Only Me
Assignee: freebsd-bugs mailing list
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-08-18 07:56 UTC by CTurt
Modified: 2018-02-08 02:45 UTC (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description CTurt 2016-08-18 07:56:55 UTC
There are some overflows when reading loader.conf, for example if you add a line like:

autoboot aaaaaaaaa...

It will overflow a 256 byte buffer here:

https://github.com/freebsd/freebsd/blob/af3e10e5a78d3af8cef6088748978c6c612757f0/sys/boot/common/boot.c#L132

https://github.com/freebsd/freebsd/blob/7fc7d2ed6e06340ab861cd094a78db87215ecff3/sys/boot/common/commands.c#L36

This sprintf pattern into command_errbuf seems to be repeated a lot, and should probably be replaced with slprintf.
Comment 1 commit-hook freebsd_committer 2016-08-20 16:23:57 UTC
A commit references this bug:

Author: tsoome
Date: Sat Aug 20 16:23:20 UTC 2016
New revision: 304532
URL: https://svnweb.freebsd.org/changeset/base/304532

Log:
  loader is filling fixed length command_errbuf with sprintf() and is trusting
  strings provided by user/config files. This update is replacing sprintf with
  snprintf for cases the command_errbuf is built from dynamic content.

  PR:		211958
  Reported by:	ecturt@gmail.com
  Reviewed by:	imp, allanjude
  Approved by:	imp (mentor), allanjude (mentor)
  Differential Revision:	https://reviews.freebsd.org/D7563

Changes:
  head/sys/boot/common/boot.c
  head/sys/boot/common/bootstrap.h
  head/sys/boot/common/commands.c
  head/sys/boot/common/interp.c
  head/sys/boot/common/ls.c
  head/sys/boot/common/module.c
  head/sys/boot/efi/loader/arch/amd64/framebuffer.c
  head/sys/boot/fdt/fdt_loader_cmd.c
Comment 2 commit-hook freebsd_committer 2018-02-08 02:45:27 UTC
A commit references this bug:

Author: kevans
Date: Thu Feb  8 02:44:25 UTC 2018
New revision: 329010
URL: https://svnweb.freebsd.org/changeset/base/329010

Log:
  MFC r303555,r303556,r303936,r303962,r304317,r304532,r305026,r305107,r305132,
  r305178,r305353,r305814,r306159,r306380,r306504: Loader fixes, 2016q3

  r303555: bcache should support reads shorter than sector size

  r303556: Improve boot loader quote parsing

  r303936: Add kernel environment variables under smbios.system

  r303962: Add the missing space between .asciz directive and opening quote
  for some lines with #ifdef BTXLDR_VERBOSE/#endif

  r304317: boot1.efi Free() should check for NULL to provide consistent
  behavior

  r304532: Replace sprintf -> snprintf for command_errbuf provisioned from
  dynamic content.

  r305026: Emulate efi_cons_poll when WaitForKey is not available

  r305107: Create a hook 'post-initialize' for people that want to define
  something to read in .conf files after all other .conf files for the purpose
  of overriding.

  r305132: Remove accidentally committed stray comment.

  r305178: bd_int13probe() should check extended info if sector info is bad

  r305353: Don't use -N to set the OMAGIC with data and text writeable and
  data not page aligned.

  r305814: ufsread: Do not cast struct direct from void *

  r306159: Consistently declare getsecs(void) with proper return type and void
  when no arguments are present.

  r306380: loader command interpreter should reset command_errmsg

  r306504: Fix a cluster of bugs in list EFI environment variables

  PR:             204602, 211958, 211958
  Relnotes:       yes ("Improve boot loader quote parsing")

Changes:
_U  stable/11/
  stable/11/sys/boot/common/bcache.c
  stable/11/sys/boot/common/boot.c
  stable/11/sys/boot/common/bootstrap.h
  stable/11/sys/boot/common/commands.c
  stable/11/sys/boot/common/interp.c
  stable/11/sys/boot/common/interp_forth.c
  stable/11/sys/boot/common/interp_parse.c
  stable/11/sys/boot/common/ls.c
  stable/11/sys/boot/common/module.c
  stable/11/sys/boot/common/ufsread.c
  stable/11/sys/boot/efi/boot1/boot1.c
  stable/11/sys/boot/efi/libefi/efi_console.c
  stable/11/sys/boot/efi/libefi/time.c
  stable/11/sys/boot/efi/libefi/time_event.c
  stable/11/sys/boot/efi/loader/arch/amd64/framebuffer.c
  stable/11/sys/boot/efi/loader/main.c
  stable/11/sys/boot/fdt/fdt_loader_cmd.c
  stable/11/sys/boot/forth/loader.4th
  stable/11/sys/boot/i386/Makefile.inc
  stable/11/sys/boot/i386/boot.ldscript
  stable/11/sys/boot/i386/boot0/Makefile
  stable/11/sys/boot/i386/boot2/Makefile
  stable/11/sys/boot/i386/btx/btx/Makefile
  stable/11/sys/boot/i386/btx/btxldr/Makefile
  stable/11/sys/boot/i386/btx/btxldr/btxldr.S
  stable/11/sys/boot/i386/cdboot/Makefile
  stable/11/sys/boot/i386/gptboot/Makefile
  stable/11/sys/boot/i386/gptzfsboot/Makefile
  stable/11/sys/boot/i386/libi386/biosdisk.c
  stable/11/sys/boot/i386/libi386/pxe.c
  stable/11/sys/boot/i386/libi386/smbios.c
  stable/11/sys/boot/i386/mbr/Makefile
  stable/11/sys/boot/i386/pmbr/Makefile
  stable/11/sys/boot/i386/pxeldr/Makefile
  stable/11/sys/boot/i386/zfsboot/Makefile
  stable/11/sys/boot/ofw/libofw/ofw_time.c
  stable/11/sys/boot/pc98/Makefile.inc
  stable/11/sys/boot/pc98/boot0/Makefile
  stable/11/sys/boot/pc98/boot2/Makefile
  stable/11/sys/boot/pc98/btx/btx/Makefile
  stable/11/sys/boot/pc98/btx/btxldr/Makefile
  stable/11/sys/boot/pc98/cdboot/Makefile
  stable/11/sys/boot/powerpc/kboot/main.c
  stable/11/sys/boot/powerpc/ps3/main.c
  stable/11/sys/boot/uboot/lib/time.c
Comment 3 commit-hook freebsd_committer 2018-02-08 02:45:31 UTC
A commit references this bug:

Author: kevans
Date: Thu Feb  8 02:44:25 UTC 2018
New revision: 329010
URL: https://svnweb.freebsd.org/changeset/base/329010

Log:
  MFC r303555,r303556,r303936,r303962,r304317,r304532,r305026,r305107,r305132,
  r305178,r305353,r305814,r306159,r306380,r306504: Loader fixes, 2016q3

  r303555: bcache should support reads shorter than sector size

  r303556: Improve boot loader quote parsing

  r303936: Add kernel environment variables under smbios.system

  r303962: Add the missing space between .asciz directive and opening quote
  for some lines with #ifdef BTXLDR_VERBOSE/#endif

  r304317: boot1.efi Free() should check for NULL to provide consistent
  behavior

  r304532: Replace sprintf -> snprintf for command_errbuf provisioned from
  dynamic content.

  r305026: Emulate efi_cons_poll when WaitForKey is not available

  r305107: Create a hook 'post-initialize' for people that want to define
  something to read in .conf files after all other .conf files for the purpose
  of overriding.

  r305132: Remove accidentally committed stray comment.

  r305178: bd_int13probe() should check extended info if sector info is bad

  r305353: Don't use -N to set the OMAGIC with data and text writeable and
  data not page aligned.

  r305814: ufsread: Do not cast struct direct from void *

  r306159: Consistently declare getsecs(void) with proper return type and void
  when no arguments are present.

  r306380: loader command interpreter should reset command_errmsg

  r306504: Fix a cluster of bugs in list EFI environment variables

  PR:             204602, 211958, 211958
  Relnotes:       yes ("Improve boot loader quote parsing")

Changes:
_U  stable/11/
  stable/11/sys/boot/common/bcache.c
  stable/11/sys/boot/common/boot.c
  stable/11/sys/boot/common/bootstrap.h
  stable/11/sys/boot/common/commands.c
  stable/11/sys/boot/common/interp.c
  stable/11/sys/boot/common/interp_forth.c
  stable/11/sys/boot/common/interp_parse.c
  stable/11/sys/boot/common/ls.c
  stable/11/sys/boot/common/module.c
  stable/11/sys/boot/common/ufsread.c
  stable/11/sys/boot/efi/boot1/boot1.c
  stable/11/sys/boot/efi/libefi/efi_console.c
  stable/11/sys/boot/efi/libefi/time.c
  stable/11/sys/boot/efi/libefi/time_event.c
  stable/11/sys/boot/efi/loader/arch/amd64/framebuffer.c
  stable/11/sys/boot/efi/loader/main.c
  stable/11/sys/boot/fdt/fdt_loader_cmd.c
  stable/11/sys/boot/forth/loader.4th
  stable/11/sys/boot/i386/Makefile.inc
  stable/11/sys/boot/i386/boot.ldscript
  stable/11/sys/boot/i386/boot0/Makefile
  stable/11/sys/boot/i386/boot2/Makefile
  stable/11/sys/boot/i386/btx/btx/Makefile
  stable/11/sys/boot/i386/btx/btxldr/Makefile
  stable/11/sys/boot/i386/btx/btxldr/btxldr.S
  stable/11/sys/boot/i386/cdboot/Makefile
  stable/11/sys/boot/i386/gptboot/Makefile
  stable/11/sys/boot/i386/gptzfsboot/Makefile
  stable/11/sys/boot/i386/libi386/biosdisk.c
  stable/11/sys/boot/i386/libi386/pxe.c
  stable/11/sys/boot/i386/libi386/smbios.c
  stable/11/sys/boot/i386/mbr/Makefile
  stable/11/sys/boot/i386/pmbr/Makefile
  stable/11/sys/boot/i386/pxeldr/Makefile
  stable/11/sys/boot/i386/zfsboot/Makefile
  stable/11/sys/boot/ofw/libofw/ofw_time.c
  stable/11/sys/boot/pc98/Makefile.inc
  stable/11/sys/boot/pc98/boot0/Makefile
  stable/11/sys/boot/pc98/boot2/Makefile
  stable/11/sys/boot/pc98/btx/btx/Makefile
  stable/11/sys/boot/pc98/btx/btxldr/Makefile
  stable/11/sys/boot/pc98/cdboot/Makefile
  stable/11/sys/boot/powerpc/kboot/main.c
  stable/11/sys/boot/powerpc/ps3/main.c
  stable/11/sys/boot/uboot/lib/time.c