[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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 redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]