Cannot compile with -Werror=format-security

[musicinmybrain]

Description of Issue

USD 21.11 cannot compile with -Werror=format-security.

Steps to Reproduce

1. Build USD with -Werror=format-security.

System Information (OS, Hardware)

Fedora Linux Rawhide (development version), x86_64.

Package Versions

21.11

Build Flags

-Werror=format-security

---

See the downstream issue for upgrading to 21.11, beginning with Comment 6.

The error is:

/builddir/build/BUILD/USD-21.11/pxr/imaging/hd/dirtyList.cpp: In member function 'void pxrInternal_v0_21__pxrReserved__::HdDirtyList::UpdateRenderTagsAndReprSelectors(const TfTokenVector&, const HdReprSelectorVector&)':
/builddir/build/BUILD/USD-21.11/pxr/imaging/hd/dirtyList.cpp:193:38: error: format not a string literal and no format arguments [-Werror=format-security]
  193 |                 TfDebug::Helper().Msg(ss.str().c_str());
      |                 ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~

The lines around the error are:

            // Reset tracked repr set.
            // XXX An alternative is to grow the tracked repr set similar to 
            //     render tags (above). This will require the render index to
            //     sync the tracked reprs rather than ones requested by the
            //     tasks.
            if (TfDebug::IsEnabled(HD_DIRTY_LIST)) {
                std::stringstream ss;
                ss << "Resetting tracked reprs in dirty list from "
                   << _trackedReprs << " to " << reprs << "\n";
                TfDebug::Helper().Msg(ss.str().c_str());
            }

For the definition of class TfDebug, see pxr/base/tf/debug.h. We find that TfDebug::Helper is a struct defined as follows:

    struct Helper {
        static TF_API void Msg(const std::string& msg);
        static TF_API void Msg(const char* msg, ...) ARCH_PRINTF_FUNCTION(1,2);
    };

The problem is that ss.str().c_str() is a char const*, so we are using the second overload of the TfDebug::Helper::Msg static method, which treats the string as a printf-style format string. This is an opportunity for havoc (https://capec.mitre.org/data/definitions/135.html), which is why it is marked with the ARCH_PRINTF_FUNCTION(1, 2) macro—which expands to __attribute__((format(printf, 1, 2))), telling GCC it is a printf-style formatting function, and triggering a warning if the format string is not a compile-time constant—and why Fedora’s default compiler flags asks GCC to treat this warning as an error (-Werror=format-security).

The fix is simple: use the std::string copy of the std::stringstream contents directly in order to use the first overload, rather than calling its c_str() method.

                TfDebug::Helper().Msg(ss.str());

I will follow up with a PR to make this change.

[musicinmybrain]

Found the same issue in HdRenderIndex::SyncAll. Will submit a PR after I get a successful test build.

[jilliene]

Filed as internal issue #USD-7015