Bug 232206 - [truss] update strsize parameter handling
Summary: [truss] update strsize parameter handling
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Only Me
Assignee: John Baldwin
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-10-12 20:01 UTC by David Carlier
Modified: 2019-01-02 20:56 UTC (History)
2 users (show)

See Also:
jhb: mfc-stable12+
jhb: mfc-stable11+


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description David Carlier 2018-10-12 20:01:06 UTC

    
Comment 1 Alan Somers freebsd_committer freebsd_triage 2018-10-12 21:20:44 UTC
A hyperlink does not a bug report make.  Even though there is a code review, you need to report the bug with as much detail as you would any other.
Comment 2 David Carlier 2018-10-12 21:45:44 UTC
Updating strsize parameter handling to set as default if the user entry is unexpected, either negative or not digits based.
Comment 3 Alan Somers freebsd_committer freebsd_triage 2018-10-12 22:00:32 UTC
Again, that's not a bug report.  That's just a broken-English description of the patch.  Bug reports need to clearly communicate what the problem is.  If you're not used to reporting bugs to open source projects, this essay is worth reading.  Learn it well, and every programmer will thank you for it.  It's not FreeBSD-specific; it applies everywhere.

https://www.chiark.greenend.org.uk/~sgtatham/bugs.html
Comment 4 David Carlier 2018-11-26 19:07:15 UTC
The bug is mainly all about fixing a possible overflow when using string size parameter, basically by giving a value over an integer maximum value.

Easily reproducible by "trussing" the syslog process with this sort of value (I just print out the value on purpose):
truss -s 27836487264287642746284662746874678412834 ...
makes the process aborting

/usr/obj/usr/src/amd64.amd64/usr.bin/truss/truss -s 23894723984789237473278482974382479238794728379843828794 -p 1509
STRSIZE -1
select(10,{ 3 5 8 9 },0x0,0x0,0x0)		 = 1 (0x1)
Bus error (core dumped)

whereas with a more "reasonable" value it works as always

/usr/obj/usr/src/amd64.amd64/usr.bin/truss/truss -s 80 -p 1553
STRSIZE 80
select(10,{ 3 5 8 9 },0x0,0x0,0x0)		 = 1 (0x1)
read(5,"Firmware Error (ACPI): Could not resolve [\\_SB.PCI0.LPCB.HEC.ECRD], AE_NOT_FOUN"...,2047) = 394 (0x18a)
writev(14,[{"Nov 26 19:06:24",15},{" ",1},{"freeflame",9},{" ",1},{"kernel",6},{": ",2},{"Firmware Error (ACPI): Could not resolve [\\_SB.PCI0.LPCB.HEC.ECRD], AE_NOT_FOUN"...,102},{"\n",1}],8) = 137 (0x89)
writev(14,[{"Nov 26 19:06:24",15},{" ",1},{"freeflame",9},{" ",1},{"kernel",6},{": ",2},{"ACPI Error: Method parse/execution failed \\_TZ.TZ00._TMP, AE_NOT_FOUND (2018103"...,93},{"\n",1}],8) = 128 (0x80)
writev(14,[{"Nov 26 19:06:24",15},{" ",1},{"freeflame",9},{" ",1},{"kernel",6},{": ",2},{"Firmware Error (ACPI): Could not resolve [\\_SB.PCI0.LPCB.HEC.ECRD], AE_NOT_FOUN"...,102},{"\n",1}],8) = 137 (0x89)
writev(14,[{"Nov 26 19:06:24",15},{" ",1},{"freeflame",9},{" ",1},{"kernel",6},{": ",2},{"ACPI Error: Method parse/execution failed \\_TZ.TZ01._TMP, AE_NOT_FOUND (2018103"...,93},{"\n",1}],8) = 128 (0x80)
read(5,0x7fffffffde50,2047)			 ERR#35 'Resource temporarily unavailable'
select(10,{ 3 5 8 9 },0x0,0x0,{ 0.000000 })	 = 0 (0x0)
fsync(0xe)					 = 0 (0x0)
select(10,{ 3 5 8 9 },0x0,0x0,0x0)		 = 1 (0x1)
...
Comment 5 commit-hook freebsd_committer freebsd_triage 2018-12-10 21:48:17 UTC
A commit references this bug:

Author: jhb
Date: Mon Dec 10 21:47:19 UTC 2018
New revision: 341802
URL: https://svnweb.freebsd.org/changeset/base/341802

Log:
  Validate the string size parameter passed to -s.

  Use strtonum() to reject negative sizes instead of core dumping.

  PR:		232206
  Submitted by:	David Carlier <devnexen@gmail.com>
  MFC after:	2 weeks
  Differential Revision:	https://reviews.freebsd.org/D17537

Changes:
  head/usr.bin/truss/main.c
Comment 6 commit-hook freebsd_committer freebsd_triage 2019-01-02 20:50:01 UTC
A commit references this bug:

Author: jhb
Date: Wed Jan  2 20:49:42 UTC 2019
New revision: 342708
URL: https://svnweb.freebsd.org/changeset/base/342708

Log:
  MFC 341802: Validate the string size parameter passed to -s.

  Use strtonum() to reject negative sizes instead of core dumping.

  PR:		232206

Changes:
_U  stable/11/
  stable/11/usr.bin/truss/main.c
_U  stable/12/
  stable/12/usr.bin/truss/main.c