byobu should not use 'ls' programmatically

Reported by Dustin Kirkland  on 2009-10-15
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
byobu
Low
Dustin Kirkland 
byobu (Debian)
Fix Released
Unknown
byobu (Ubuntu)
Low
Dustin Kirkland 

Bug Description

On Wed, 2009-09-23 at 12:35 +1000, Trent W. Buck wrote:
Package: byobu
> Version: 2.24-1
> Severity: normal
> File: /usr/lib/byobu/battery
>
> I get
>
> ls: cannot access /proc/acpi/battery: No such file or directory
>
> because on my system, this information is computed thusly:
>
> backtick 2 120 0 sh -c 'cd /sys/class/power_supply/BAT0 && echo \ $((100 * `cat charge_now` / `cat charge_full`))%'
>
> It is also bad form to use ls(1) programmatically; you should use
> globbing instead: http://mywiki.wooledge.org/ParsingLs

Related branches

Changed in byobu:
status: New → Triaged
importance: Undecided → Low
assignee: nobody → Dustin Kirkland (kirkland)
Changed in byobu (Debian):
status: Unknown → New
codeslinger (codeslinger) wrote :

not sure if this is relevant, but on my system BAT0 does not exist and neither does "charge_now"

perhaps the folder path is os version specific and I'm off-track??? but I would not expect something fundamental to be arbitrary.

I'm running ubuntu hardy 8.04 on a Toshiba Laptop, and what I have is:

/sys/class/power_supply/
    ADP1
    BAT1

and inside of BAT1 instead of "charge_" everything says "energy_"

/sys/class/power_supply/BAT1
    energy_full
    energy_now

while I agree with the bug poster about the dangers of parsing LS and enjoyed reading the very well written article that he points to.

In this scenario we are dealing with well-known names provided by the system. so I think it is safe in this specific scenario to parse a name when we already know in advance what that name should be.

The only question becomes one of security, if someone put a bogus name with a newline to fool us, would anything bad happen? I suspect that the answer is the worse that could happen is an error message.

Changed in byobu:
status: Triaged → In Progress
Changed in byobu (Ubuntu):
status: New → In Progress
importance: Undecided → Low
assignee: nobody → Dustin Kirkland (kirkland)
Changed in byobu:
status: In Progress → Fix Committed
Changed in byobu (Ubuntu):
status: In Progress → Fix Committed
Changed in byobu:
status: Fix Committed → Fix Released
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package byobu - 2.46-0ubuntu1

---------------
byobu (2.46-0ubuntu1) lucid; urgency=low

  * byobu-janitor: seed the default color choices more effciently, also
    fixes some strange crashes on first run; default the colors to white
    on black
  * byobu-config, doc/help.txt: insert version number in byobu help menu,
    LP: #502119
  * bin/network: match interface a bit better for bridges, LP: #498722
  * bin/logo, bin/release: when looking at /etc/issue, grab the first
    line that starts with a real letter rather than control character,
    LP: #432751
  * byobu: deprecate code that prevented nested byobu sessions; this is
    is actually quite useful, although you should choose different
    escape sequences for each nested host, LP: #403988
  * bin/battery, bin/custom, bin/fan_speed, bin/temp, bin/temp_f: don't
    use ls programatically in for-loops, LP: #452405
  * bin/cpu_temp, bin/temp_f, bin/temp_c, byobu-config, byobu.1,
    profiles/common, rpm/byobu.spec, statusrc: merge temp_f and temp_c
    into a single cpu_temp script
  * profiles/common: adjust the formatting-only, and run-only-at-startup
    status scripts to run every 9,999,999 seconds (~115 days), minor
    performance improvement once a day, easier to identify such jobs;
    adjust cpu_count and mem_available to run more frequently, as
    cpu-hotplug and memory-ballooning are becoming more feasible,
    particularly in VMs
 -- Dustin Kirkland <email address hidden> Thu, 07 Jan 2010 21:53:14 -0600

Changed in byobu (Ubuntu):
status: Fix Committed → Fix Released
Changed in byobu (Ubuntu):
status: Fix Released → Fix Committed
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package byobu - 2.63-0ubuntu1

---------------
byobu (2.63-0ubuntu1) lucid; urgency=low

  [ Dustin Kirkland ]
  * etc/byobu/socketdir, usr/bin/byobu-config, usr/bin/byobu-janitor,
    usr/bin/byobu-select-profile: increase portability to other distros,
    create socketdir conffile and SOCKETDIR variable for users who don't
    use /var/run/screen as the socket directory, LP: #535378
  * usr/bin/byobu-config: change a sample reference from "bash" to "shell"
    again for portability, LP: #535407
  * bin/byobu-export, bin/byobu-reconnect-sockets, bin/byobu-status,
    lib/byobu/cpu_temp: avoid programmatic use of ls in for loops,
    LP: #452405

  [ Torsten Spindler ]
  * debian/postinst: handle situation where /var/run/screen does not
    exist, LP: #535648
 -- Dustin Kirkland <email address hidden> Tue, 09 Mar 2010 14:53:10 -0600

Changed in byobu (Ubuntu):
status: Fix Committed → Fix Released
Changed in byobu (Debian):
status: New → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.