Incorrect calculation of string lengths

Bug #1558109 reported by Daniel on 2016-03-16
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
live-build (Ubuntu)
Medium
Unassigned

Bug Description

When calling lb_config with parameters like `--iso-volume`, the length of the string is not correctly determined. The argument in the following example is limited to 32 characters, but the code in `/usr/share/live/build/functions/defaults.sh` only allows 31 characters:

# trying with 33 characters -> fails as expected
root@53e16c910d1d:/tmp# lb config --iso-volume '123456789-123456789-123456789-123'
[2016-03-16 15:21:05] lb_config --iso-volume 123456789-123456789-123456789-123
P: Considering defaults defined in /etc/live/build.conf
W: You have specified a value of LB_ISO_VOLUME that is too long; the maximum length is 32 characters.
P: Updating config tree for a ubuntu/amd64 system
W: You have specified a value of LB_ISO_VOLUME that is too long; the maximum length is 32 characters.

# trying with 32 characters -> fails but should work
root@53e16c910d1d:/tmp# lb config --iso-volume '123456789-123456789-123456789-12'
[2016-03-16 15:21:07] lb_config --iso-volume 123456789-123456789-123456789-12
P: Considering defaults defined in /etc/live/build.conf
W: You have specified a value of LB_ISO_VOLUME that is too long; the maximum length is 32 characters.
P: Updating config tree for a ubuntu/amd64 system
W: You have specified a value of LB_ISO_VOLUME that is too long; the maximum length is 32 characters.

# trying with 31 characters -> works as expected
root@53e16c910d1d:/tmp# lb config --iso-volume '123456789-123456789-123456789-1'
[2016-03-16 15:21:09] lb_config --iso-volume 123456789-123456789-123456789-1
P: Considering defaults defined in /etc/live/build.conf
P: Updating config tree for a ubuntu/amd64 system

There are two reasons for this error:
 * The string is sent to `wc -c` using `echo` which adds an extra line end character to the string
 * three of four tests surround their string by escaped " characters which are also counted. For LB_ISO_VOLUME, the developer tried to fix this using the `eval` function

Additionally, the code is unnecessarily complex. Both dash and bash provide the `$#variable` syntax for getting the string length

I add two patches, one fixes the problem and the second one contains the (simpler) alternative alternative syntax which works similar.

Tested with live-build from 3.0~a57-1ubuntu22 from ubuntu wily but the current 3.0~a57-1ubuntu25 from xenial contains the same code

root@53e16c910d1d:/# dpkg -l live-build
Desired=Unknown/Install/Remove/Purge/Hold
| Status=Not/Inst/Conf-files/Unpacked/halF-conf/Half-inst/trig-aWait/Trig-pend
|/ Err?=(none)/Reinst-required (Status,Err: uppercase=bad)
||/ Name Version Architecture Description
+++-===========================================================-==================================-==================================-===========================================================================================================================
ii live-build 3.0~a57-1ubuntu22 all Debian Live - System build scripts

root@53e16c910d1d:/# lsb_release -a
No LSB modules are available.
Distributor ID: Ubuntu
Description: Ubuntu 15.10
Release: 15.10
Codename: wily

Console output of patched code (both variants):

root@53e16c910d1d:/tmp# lb config --iso-volume '123456789-123456789-123456789-123'
[2016-03-16 15:20:26] lb_config --iso-volume 123456789-123456789-123456789-123
P: Considering defaults defined in /etc/live/build.conf
W: You have specified a value of LB_ISO_VOLUME that is too long; the maximum length is 32 characters.
P: Updating config tree for a ubuntu/amd64 system
W: You have specified a value of LB_ISO_VOLUME that is too long; the maximum length is 32 characters.

root@53e16c910d1d:/tmp# lb config --iso-volume '123456789-123456789-123456789-12'
[2016-03-16 15:20:29] lb_config --iso-volume 123456789-123456789-123456789-12
P: Considering defaults defined in /etc/live/build.conf
P: Updating config tree for a ubuntu/amd64 system

Daniel (hackie) wrote :

The attachment "variant1.patch" seems to be a patch. If it isn't, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are a member of the ~ubuntu-reviewers, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issues please contact him.]

tags: added: patch
Daniel (hackie) wrote :

Correction: I just realized that the uploaded variant1 is the simpler version and variant2 the simple bugfix.

Changed in live-build (Ubuntu):
importance: Undecided → Medium
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers