On Fri, Sep 03 2021, Renaud Allard <ren...@allard.it> wrote:
> On 9/2/21 11:38 PM, Jeremie Courreges-Anglas wrote:
>> On Thu, Sep 02 2021, Jeremie Courreges-Anglas <j...@wxcvbn.org> wrote:
>>> On Thu, Sep 02 2021, "Theo de Raadt" <dera...@openbsd.org> wrote:
>>>> Jeremie Courreges-Anglas <j...@wxcvbn.org> wrote:
>>>>
>>>>>
>>>>>
>>>>> exim apparently uses printf("%n"), which is currently forbidden (libc
>>>>> calls abort(3)).
>>>>>
>>>>> I don't want us to fix all the %n uses in the ports tree, but instead
>>>>> wait for user reports. Though for some software like exim it makes
>>>>> sense to help users avoid such a hard crash.
>>>>>
>>>>> The diff below doesn't pretend to fix all uses of %n in the exim source.
>>>>> There may be others that can't be flagged by the compiler (support for
>>>>> that hesn't been committed yet) because of indirections through wrapper
>>>>> functions.
>>>>> +--- src/acl.c.orig
>>>>> ++++ src/acl.c
>>>>> +@@ -2906,10 +2906,12 @@ for (; cb; cb = cb->next)
>>>>> +
>>>>> + HDEBUG(D_acl)
>>>>> + {
>>>>> +- int lhswidth = 0;
>>>>> +- debug_printf_indent("check %s%s %n",
>>>>> ++ uschar buf[256];
>>>>> ++ int lhswidth = snprintf(CS buf, sizeof buf, "check %s%s ",
>>>>> + (!conditions[cb->type].is_modifier && cb->u.negated)? "!":"",
>>>>> +- conditions[cb->type].name, &lhswidth);
>>>>> ++ conditions[cb->type].name);
>>>>> ++ if (lhswidth == -1) lhswidth = 0;
>>>>> ++ debug_printf_indent("%s");
>>>>
>>>> Doesn't this %s need an argument buf?
>>>
>>> Urkh, indeed, thanks. New diff below.
>> Theo pointed out another problem in one of the two changes:
>>
>>> + uschar * s = string_sprintf("Return-path: <%.*s>\n%n",
>> Here I forgot to actually drop the %n. clang still accepted it,
>> probably because this function isn't marked as "printf-like".
>> I'm not proposing another diff right now because I'm busy with other
>> stuff, I'll get back to this when I have a clear mind.
>>
>
> Actually, it seems there is more than one:
> src/transport.c: uschar * s = string_sprintf("Return-path: <%.*s>\n%n",
> src/rewrite.c: new = string_sprintf("%.*s%s%n%s",
If I read correctly the source code, the formatting by string_sprintf
and debug_printf_indent seem to be implemented by code in src/string.c,
not by our libc formatting code. Therefore exim shouldn't reach the
fatal abort(3) in our libc, therefore I'm dropping the whole diff.
I suggest that you verify this claim by testing test exim on a -current
system where printf("%n") aborts.
Don't get me wrong, I'm not saying that those uses of %n are legit.
They should be fixed eventually, and getting rid of this duplicated
formatting code might be a good idea. But I'm not volunteering.
Sorry for the noise,
--
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
