Re: [libvirt] [PATCH] maint: don't permit format strings without %
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(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 620 bytes)