[libvirt] [PATCH] maint: don't permit format strings without %

[Eric Blake]

On 07/23/2012 03:05 PM, Jim Meyering wrote:
> Eric Blake wrote:
>> Any time we have a string with no % passed through gettext, a
>> translator can inject a % to cause a stack overread.  When there
>> is nothing to format, it's easier to ask for a string that cannot
>> be used as a formatter, by using a trivial "%s" format instead.
>>
>> In the past, we have used --disable-nls to catch some of the
>> offenders, but that doesn't get run very often, and many more
>> uses have crept in.  Syntax check to the rescue!
>>

>>
>> -func_or := $(shell printf '$(msg_gen_function)'|tr -s '[[:space:]]' '|')
>> +func_or := $(shell echo $(msg_gen_function)|tr -s '[[:space:]]' '|')

At this point in time, $(msg_gen_function) was built via:

msg_gen_function =
msg_gen_function += VIR_ERROR
msg_gen_function += ...

which means it has the literal value ' VIR_ERROR ...'.

The quoted printf version ends up converting the literal leading space
into '|', giving a regex (|VIR_ERROR|...) for $(func_re) which matches
_everything_, when used with no further anchors.  Thankfully, we were
always using $(func_re) with a preceding anchor of \<, and the current
glibc implementation of \< followed by an empty regex doesn't match
(although I don't know if that was intentional).

I switched over to an unquoted printf, so as to let the shell do IFS
splitting and thus eat the leading space.


> Nice patch.
> The subject piqued my interest, so even on PTO, I took a quick look.
> One issue (I haven't gone through the whole thing)
> is that you should not insert the new rule there, because
> by separating the preceding and following rules, it would
> render invalid the following "Like the above..." comment.

Sure, I can reorder things.

> 
> Also, I didn't see right off why you've changed from using
> printf to using echo in the definition of func_or.
> Without looking at the definition of msg_gen_function, I
> suspect that switching to echo will add an unwanted trailing "|", no?
> Seeing no ChangeLog entry, I wondered if it was an accident.

Intentional, but probably worth an independent commit since I had to
justify it above.

-- 
Eric Blake   eblake at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120724/b5d2a8b4/attachment-0001.sig>