On Mon, Jul 07, 2025 at 07:06:06AM +0200, Alejandro Colomar wrote:
> I've written an analysis of snprintf(3), why it's dangerous, and how
> these APIs address that, and will present it as a proposal for
> standardization of these APIs in ISO C2y.  I'll send that as a reply to
> this message in a moment, as I believe it will be interesting for
> linux-hardening@.

Hi,

Here is the proposal for ISO C2y (see below).  I'll also send it to the
C Committee for discussion. 


Have a lovely night!
Alex

---
Name
        alx-0049r1 - add seprintf()

Principles
        -  Codify existing practice to address evident deficiencies.
        -  Enable secure programming

Category
        Standardize existing libc APIs

Author
        Alejandro Colomar <a...@kernel.org>

        Cc: Christopher Bazley <chris.bazley.w...@gmail.com>

History
        <https://www.alejandro-colomar.es/src/alx/alx/wg14/alx-0049.git/>

        r0 (2025-07-06):
        -  Initial draft.

        r1 (2025-07-06):
        -  wfix.
        -  tfix.
        -  Expand on the off-by-one bugs.
        -  Note that ignoring truncation is not valid most of the time.

Rationale
        snprintf(3) is very difficult to chain for writing parts of a
        string in separate calls, such as in a loop.

        Let's start from the obvious sprintf(3) code (sprintf(3) will
        not prevent overflow, but let's take it as a baseline from which
        programmers start thinking):

                p = buf;
                for (...)
                        p += sprintf(p, ...);

        Then, programmers will start thinking about preventing buffer
        overflows.  Programmers sometimes will naively add some buffer
        size information and use snprintf(3):

                p = buf;
                size = countof(buf);
                for (...)
                        p += snprintf(p, size - (p - buf), ...);

                if (p >= buf + size)  // or worse, (p > buf + size - 1)
                        goto fail;

        (Except for minor differences, this kind of code can be found
         everywhere.  Here are a couple of examples:
         <https://elixir.bootlin.com/linux/v6.14/source/mm/slub.c#L7231>
         <https://elixir.bootlin.com/linux/v6.14/source/mm/mempolicy.c#L3369>.)

        This has several issues, starting with the difficulty of getting
        the second argument right.  Sometimes, programmers will be too
        confused, and slap a -1 there just to be safe.

                p = buf;
                size = countof(buf);
                for (...)
                        p += snprintf(p, size - (p - buf) - 1, ...);

                if (p >= buf + size -1)
                        goto fail;

        (Except for minor differences, this kind of code can be found
         everywhere.  Here are a couple of examples:
         
<https://elixir.bootlin.com/linux/v6.14/source/mm/kfence/kfence_test.c#L113>
         
<https://elixir.bootlin.com/linux/v6.14/source/mm/kmsan/kmsan_test.c#L108>.)

        Programmers will sometimes hold a pointer to one past the last
        element in the array.  This is a wise choice, as that pointer is
        constant throughout the lifetime of the object.  Then,
        programmers might end up with something like this:

                p = buf;
                e = buf + countof(buf);
                for (...)
                        p += snprintf(p, e - p, ...);

                if (p >= end)
                        goto fail;

        This is certainly much cleaner.  Now a programmer might focus on
        the fact that this can overflow the pointer.  An easy approach
        would be to make sure that the function never returns more than
        the remaining size.  That is, one could implement something like
        this scnprintf() --name chosen to match the Linux kernel API of
        the same name--.  For the sake of simplicity, let's ignore
        multiple evaluation of arguments.

                #define scnprintf(s, size, ...)                 \
                ({                                              \
                        int len_;                               \
                        len_ = snprintf(s, size, __VA_ARGS__);  \
                        if (len_ == -1)                         \
                                len_ = 0;                       \
                        if (len_ >= size)                       \
                                len_ = size - 1;                \
                                                                \
                        len_;                                   \
                })

                p = buf;
                e = buf + countof(buf);
                for (...)
                        p += scnprintf(p, e - p, ...);

        (Except for minor differences, this kind of code can be found
         everywhere.  Here's an example:
         
<https://elixir.bootlin.com/linux/v6.14/source/mm/kfence/kfence_test.c#L131>.)

        Now the programmer got rid of pointer overflow.  However, they
        now have silent truncation that cannot be detected.  In some
        cases this may seem good enough.  However, often it's not.  And
        anyway, some code remains using snprintf(3) to be able to detect
        truncation.

        Moreover, this kind of code ignores the fact that vsnprintf(3)
        can fail internally, in which case there's not even a truncated
        string.  In the kernel, they're fine, because their internal
        vsnprintf() doesn't seem to ever fail, so they can always rely
        on the truncated string.  This is not reliable in projects that
        rely on the libc vsnprintf(3).

        For the code that needs to detect truncation, a programmer might
        choose a different path.  It would keep using snprintf(3), but
        would use a temporary length variable instead of the pointer.

                p = buf;
                e = buf + countof(buf);
                for (...) {
                        len = snprintf(p, e - p, ...);
                        if (len == -1)
                                goto fail;
                        if (len >= e - p)
                                goto fail;
                        p += len;
                }

        This is naturally error-prone.  A colleague of mine --which is an
        excellent programmer, to be clear--, had a bug even after
        knowing about it and having tried to fix it.  That shows how
        hard it is to write this correctly:
        <https://github.com/nginx/unit/pull/734#discussion_r1043963527>

        In a similar fashion, the strlcpy(3) manual page from OpenBSD
        documents a similar issue when chaining calls to strlcpy(3)
        --which was designed with semantics equivalent to snprintf(3),
        except for not formatting the string--:

        |            char *dir, *file, pname[MAXPATHLEN];
        |            size_t n;
        |
        |            ...
        |
        |            n = strlcpy(pname, dir, sizeof(pname));
        |            if (n >= sizeof(pname))
        |                    goto toolong;
        |            if (strlcpy(pname + n, file, sizeof(pname) - n) >= 
sizeof(pname) - n)
        |                    goto toolong;
        |
        |       However, one may question the validity of such optimiza‐
        |       tions, as they defeat the whole purpose of strlcpy() and
        |       strlcat().  As a matter of fact, the  first  version  of
        |       this manual page got it wrong.

        Finally, a programmer might realize that while this is error-
        prone, this is indeed the right thing to do.  There's no way to
        avoid it.  One could then think of encapsulating this into an
        API that at least would make it easy to write.  Then, one might
        wonder what the right parameters are for such an API.  The only
        immutable thing in the loop is 'e'.  And apart from that, one
        needs to know where to write, which is 'p'.  Let's start with
        those, and try to keep all the other information (size, len)
        without escaping the API.  Again, let's ignore multiple-
        evaluation issues in this macro for the sake of simplicity.

                #define foo(p, e, ...)                                \
                ({                                                    \
                        int  len_ = snprintf(p, e - p, __VA_ARGS__);  \
                        if (len_ == -1)                               \
                                p = NULL;                             \
                        else if (len_ >= e - p)                       \
                                p = NULL;                             \
                        else                                          \
                                p += len_;                            \
                        p;
                })

                p = buf;
                e = buf + countof(buf);
                for (...) {
                        p = foo(p, e, ...);
                        if (p == NULL)
                                goto fail;
                }

        We've advanced a lot.  We got rid of the buffer overflow; we
        also got rid of the error-prone code at call site.  However, one
        might think that checking for truncation after every call is
        cumbersome.  Indeed, it is possible to slightly tweak the
        internals of foo() to propagate errors from previous calls.

                #define seprintf(p, e, ...)                           \
                ({                                                    \
                        if (p != NULL) {                              \
                                int  len_;                            \
                                                                      \
                                len_ = snprintf(p, e - p, __VA_ARGS__); \
                                if (len_ == -1)                       \
                                        p = NULL;                     \
                                else if (len_ >= e - p)               \
                                        p = NULL;                     \
                                else                                  \
                                        p += len_;                    \
                        }                                             \
                        p;                                            \
                })

                p = buf;
                e = buf + countof(buf);
                for (...)
                        p = seprintf(p, e, ...);

                if (p == NULL)
                        goto fail;

        By propagating an input null pointer directly to the output of
        the API, which I've called seprintf() --the 'e' refers to the
        'end' pointer, which is the key in this API--, we've allowed
        ignoring null pointers until after the very last call.  If we
        compare our resulting code to the sprintf(3)-based baseline, we
        got --perhaps unsurprisingly-- something quite close to it:

                p = buf;
                for (...)
                        p += sprintf(p, ...);

        vs

                p = buf;
                e = buf + countof(buf);
                for (...)
                        p = seprintf(p, e, ...);

                if (p == NULL)
                        goto fail;

        And the seprintf() version is safe against both truncation and
        buffer overflow.

        Some important details of the API are:

        -  When 'p' is NULL, the API must preserve errno.  This is
           important to be able to determine the cause of the error
           after all the chained calls, even when the error occurred in
           some call in the middle of the chain.

        -  When truncation occurs, a distinct errno value must be used,
           to signal the programmer that at least the string is reliable
           to be used as a null-terminated string.  The error code
           chosen is E2BIG, for compatibility with strscpy(), a Linux
           kernel internal API with which this API shares many features
           in common.

        -  When a hard error (an internal snprintf(3) error) occurs, an
           error code different than E2BIG must be used.  It is
           important to set errno, because if an implementation would
           chose to return NULL without setting errno, an old value of
           E2BIG could lead the programmer to believe the string was
           successfully written (and truncated), and read it with
           nefast consequences.

Prior art
        This API is implemented in the shadow-utils project.

        Plan9 designed something quite close, which they call
        seprint(2).  The parameters are the same --the right choice--,
        but they got the semantics for corner cases wrong.  Ironically,
        the existing Plan9 code I've seen seems to expect the semantics
        that I chose, regardless of the actual semantics of the Plan9
        API.  This is --I suspect--, because my semantics are actually
        the intuitive semantics that one would naively guess of an API
        with these parameters and return value.

        I've implemented this API for the Linux kernel, and found and
        fixed an amazing amount of bugs and other questionable code in
        just the first handful of files that I inspected.
        
<https://lore.kernel.org/linux-hardening/cover.1751747518.git....@kernel.org/T/#t>
        
<https://lore.kernel.org/linux-hardening/cover.1751823326.git....@kernel.org/T/#t>

Future directions
        The 'e = buf + _Countof(buf)' construct is something I've found
        to be quite common.  It would be interesting to have an
        _Endof operator that would return a pointer to one past the last
        element of an array.  It would require an array operand, just
        like _Countof.  If an _Endof operator is deemed too cumbersome
        for implementation, an endof() standard macro that expands to
        the obvious implementation with _Countof could be okay.

        This operator (or operator-like macro) would prevent off-by-one
        bugs when calculating the end sentinel value, such as those
        shown above (with links to Linux kernel real bugs).

Proposed wording
        Based on N3550.

    7.24.6  Input/output <stdio.h> :: Formatted input/output functions
        ## New section after 7.24.6.6 ("The snprintf function"):

        +7.24.6.6+1  The <b>seprintf</b> function
        +
        +Synopsis
        +1      #include <stdio.h>
        +       char *seprintf(char *restrict p, const char end[0], const char 
*restrict format, ...);
        +
        +Description
        +2      The <b>$0</b> function
        +       is equivalent to <b>fprintf</b>,
        +       except that the output is written into an array
        +       (specified by argument <tt>p</tt>)
        +       rather than a stream.
        +       If <tt>p</tt> is a null pointer,
        +       nothing is written,
        +       and the function returns a null pointer.
        +       Otherwise,
        +       <tt>end</tt> shall compare greater than <tt>p</tt>;
        +       the function writes at most
        +       <tt>end - p - 1</tt> non-null characters,
        +       the remaining output characters are discarded,
        +       and a null character is written
        +       at the end of the characters
        +       actually written to the array.
        +       If copying takes place between objects that overlap,
        +       the behavior is undefined.
        +
        +Returns
        +3      The <b>$0</b> function returns
        +       a pointer to the terminating null character
        +       if the output was written
        +       without discarding any characters.
        +
        +4
        +       If <tt>p</tt> is a null pointer,
        +       a null pointer is returned,
        +       and <b>errno</b> is not modified.
        +
        +5
        +       If any characters are discarded,
        +       a null pointer is returned,
        +       and the value of the macro <b>E2BIG</b>
        +       is stored in <b>errno</b>.
        +
        +6
        +       If an error occurred,
        +       a null pointer is returned,
        +       and an implementation-defined non-zero value
        +       is stored in <b>errno</b>.

        ## New section after 7.24.6.13 ("The vsnprintf function"):

        +7.24.6.13+1  The <b>vseprintf</b> function
        +
        +Synopsis
        +1      #include <stdio.h>
        +       char *vseprintf(char *restrict p, const char end[0], const char 
*restrict format, va_list arg);
        +
        +Description
        +2      The <b>$0</b> function
        +       is equivalent to
        +       <b>seprintf</b>,
        +       with the varying argument list replaced by <tt>arg</tt>.
        +
        +3
        +       The <tt>va_list</tt> argument to this function
        +       shall have been initialized by the <b>va_start</b> macro
        +       (and possibly subsequent <b>va_arg</b> invocations).
        +       This function does not invoke the <b>va_end</b> macro.343)

    7.33.2  Formatted wide character input/output functions
        ## New section after 7.33.2.4 ("The swprintf function"):

        +7.33.2.4+1  The <b>sewprintf</b> function
        +
        +Synopsis
        +1      #include <wchar.h>
        +       wchar_t *sewprintf(wchar_t *restrict p, const wchar_t end[0], 
const wchar_t *restrict format, ...);
        +
        +Description
        +2      The <b>$0</b> function
        +       is equivalent to
        +       <b>seprintf</b>,
        +       except that it handles wide strings.

        ## New section after 7.33.2.8 ("The vswprintf function"):

        +7.33.2.8+1  The <b>vsewprintf</b> function
        +
        +Synopsis
        +1      #include <wchar.h>
        +       wchar_t *vsewprintf(wchar_t *restrict p, const wchar_t end[0], 
const wchar_t *restrict format, va_list arg);
        +
        +Description
        +2      The <b>$0</b> function
        +       is equivalent to
        +       <b>sewprintf</b>,
        +       with the varying argument list replaced by <tt>arg</tt>.
        +
        +3
        +       The <tt>va_list</tt> argument to this function
        +       shall have been initialized by the <b>va_start</b> macro
        +       (and possibly subsequent <b>va_arg</b> invocations).
        +       This function does not invoke the <b>va_end</b> macro.407)

-- 
<https://www.alejandro-colomar.es/>

Attachment: signature.asc
Description: PGP signature

Reply via email to