builtins/help.def: Passes ngettext() result to printf() as format string

Bug #1644048 reported by Julian Andres Klode on 2016-11-22
12
This bug affects 2 people
Affects Status Importance Assigned to Milestone
bash (Ubuntu)
High
Unassigned
Trusty
High
Unassigned

Bug Description

[Impact]
Breaks build on arm64 in trusty:

../.././builtins/../.././builtins/help.def:130:7: error: format not a string literal and no format arguments [-Werror=format-security]

[Test case]
Check it builds

[Regression potential]
Indefinitely low. All we do is add
  "%s",
between printf( and ngettext(...

[Other info]
The same code works fine on all other architectures and newer releases, but it seems broken anyway: We are passing the return value of ngettext() to printf() as the format string, which is unsafe.

We should evaluate why that works elsewhere and probably also do the same fix in other branches, but I'll leave that to someone else to decide. My intention here is to just get the trusty SRU for bug 1644048 building on all platforms.

Julian Andres Klode (juliank) wrote :
Julian Andres Klode (juliank) wrote :

Apparently zesty has the same code in it, but it weirdly works there.

summary: - 4.3-7ubuntu1.6 FTBFS on arm64
+ 4.3-7ubuntu1.6 FTBFS on arm64 only with format-security error
tags: added: patch

On the suggested fix: Why even keep the printf? Why not switch to just print?
I'm not a C expert by any means, but shouldn't
printf("%s", <something returning a string>)
be completely equivalent to
print(<something returning a string>)
With the exception that the second one should be very slightly less overhead?

Launchpad Janitor (janitor) wrote :

Status changed to 'Confirmed' because the bug affects multiple users.

Changed in bash (Ubuntu Trusty):
status: New → Confirmed
Changed in bash (Ubuntu):
status: New → Confirmed
Julian Andres Klode (juliank) wrote :

No, there is no such print() equivalent. You have puts() which just takes a string, but that appends a newline. Or fwrite(), but that requires size and count, which is boring.

description: updated
Changed in bash (Ubuntu Trusty):
status: Confirmed → In Progress
description: updated
description: updated
Julian Andres Klode (juliank) wrote :

SRU was rejected. See bug 1644363 for the actual cause of the issue (hint: ld crashes during static configure).

We might still want to get that fixed in new releases though, but I'm unsure if this needs updates in stable releases (maybe it improves security, who knows...).

Changed in bash (Ubuntu Trusty):
status: In Progress → Confirmed
summary: - 4.3-7ubuntu1.6 FTBFS on arm64 only with format-security error
+ builtins/help.def: Passes ngettext() result to printf() as format string
tags: added: ftbfs
Changed in bash (Ubuntu Trusty):
importance: Undecided → High
Changed in bash (Ubuntu):
importance: Undecided → High
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers