Bug 204602 - parse() in boot loader interp_parse.c is too naive about quotes
Summary: parse() in boot loader interp_parse.c is too naive about quotes
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Some People
Assignee: Kyle Evans
URL:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2015-11-16 17:13 UTC by Toomas Soome
Modified: 2018-02-08 03:19 UTC (History)
2 users (show)

See Also:
kevans: mfc-stable11+


Attachments
udiff of inter_parse.c (1.63 KB, patch)
2015-11-16 17:13 UTC, Toomas Soome
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Toomas Soome 2015-11-16 17:13:35 UTC
Created attachment 163200 [details]
udiff of inter_parse.c

current logic how the quotes (both ' and ") are managed is a bit too relaxed, allowing wierd constructs like set name="value' also usual single quote semantics is not possible and, the code does not check if the quoted string actually has ending quote.

I'm adding here diff for possible update, which implements:

1. distinguishing single and double quote
2. variable expansion will not be done inside single quote protected area
3. will preserve inner quote for values like "value 'some list'"
4. ending quote check.

however, this diff does not implement ending quote order check - it shouldn't be too hard, needs some improvements on parser state machine.
Comment 1 commit-hook freebsd_committer freebsd_triage 2016-07-30 17:54:39 UTC
A commit references this bug:

Author: allanjude
Date: Sat Jul 30 17:53:37 UTC 2016
New revision: 303556
URL: https://svnweb.freebsd.org/changeset/base/303556

Log:
  Improve boot loader quote parsing

  parse() is the boot loader's interp_parse.c is too naive about quotes

  both single and double quotes were allowed to be mixed, and single
  quotes did not follow the usual semantics (re variable expansion).

  The old code did not check for terminating quotes

  This update implements:
   * distinguishing single and double quote
   * variable expansion will not be done inside single quote protected area
   * will preserve inner quote for values like "value 'some list'"
   * ending quote check.

  this diff does not implement ending quote order check, it shouldn't
  be too hard, needs some improvements on parser state machine.

  PR:		204602
  Submitted by:	Toomas Soome <tsoome@me.com>
  Relnotes:	yes
  Differential Revision:	https://reviews.freebsd.org/D6000

Changes:
  head/sys/boot/common/interp_parse.c
Comment 2 commit-hook freebsd_committer freebsd_triage 2018-02-08 02:45:29 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 Kyle Evans freebsd_committer freebsd_triage 2018-02-08 03:19:30 UTC
I'm going to go ahead and close- MFC'd to stable/11.