[libvirt] [PATCH] maint: don't permit format strings without %
eblake at redhat.com
Tue Jul 24 19:45:42 UTC 2012
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 += 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...
Size: 620 bytes
Desc: OpenPGP digital signature
More information about the libvir-list