Log4cxx
  1. Log4cxx
  2. LOGCXX-162

Problem printing string with embedded NULL character

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.10.0
    • Component/s: None
    • Labels:
      None
    • Environment:
      log4cxx-svn-427970
      linux / gcc

      Description

      It's exactly the problem described in:
      http://www.mail-archive.com/log4cxx-user@logging.apache.org/msg01650.html

      ---- COPY OF MESSAGE ----

      Problem printing string with embedded NULL character

      Venkatraju
      Sat, 29 Jul 2006 19:30:11 -0700

      Hi,

      Printing a string with embedded NULL character causes
      MbstowcsCharsetDecoder::decode to go into an infinite loop trying to
      consume that character.

      Test case below:
      ------
      #include <iostream>
      #include <log4cxx/basicconfigurator.h>

      using namespace log4cxx;

      int main()

      { BasicConfigurator::configure(); std::string badStr; badStr.append("hello"); badStr.append(1, '\0'); badStr.append("world"); LOG4CXX_ERROR(Logger::getRootLogger(), badStr); return 0; } --------- Fixed with this patch: --- Index: src/charsetdecoder.cpp =================================================================== --- src/charsetdecoder.cpp (revision 390980) +++ src/charsetdecoder.cpp (working copy) @@ -178,6 +178,12 @@ stat = append(out, buf); in.position(in.position() + converted); }

      +
      + if (in.remaining() > 0 && (src == 0))

      { + // Found embedded NULL in string + stat = APR_BADARG; + break; + }

      }
      return stat;
      }


      Can someone merge this into the log4cxx trunk? Please let me know if I
      need to provide more info.

      Thanks,
      Venkat

        Activity

        Hide
        Curt Arnold added a comment -

        Committed rev 428399.

        log4cxx's CharsetDecoder mimics java.nio.charset.CharsetDecoder. It wasn't obvious how it would handle an embedded null character, so I wrote the following unit test to determine its behavior:

        public void testDecode8() {
        final byte[] buf = new byte[]

        { 'H', 'e', 'l', 'l', 'o', ',', 0, 'W', 'o', 'r', 'l', 'd'}

        ;
        ByteBuffer src = ByteBuffer.wrap(buf);

        CharsetDecoder dec = Charset.forName("ISO-8859-1").newDecoder();

        CharBuffer greeting = CharBuffer.allocate(20);
        CoderResult stat = dec.decode(src, greeting, true);
        assertSame(CoderResult.UNDERFLOW, stat);

        stat = dec.decode(src, greeting, true);
        assertSame(CoderResult.UNDERFLOW, stat);

        assertEquals(12, src.position());
        assertEquals("Hello,\u0000World", greeting.flip().toString());
        }

        If appears that if a null byte is encountered in the source, a null character is decoded. It does not return an error condition like the patch intended to do. The patch adds a C++ equivalent of the previous test and modified the MbstowcsCharsetDecoder::decode implementation to avoid calling mbstowcs when the string starts with a null character. It is possible that the same weakness exists in the other decoders, hopefully the unit test will catch those and they can be corrected.

        Show
        Curt Arnold added a comment - Committed rev 428399. log4cxx's CharsetDecoder mimics java.nio.charset.CharsetDecoder. It wasn't obvious how it would handle an embedded null character, so I wrote the following unit test to determine its behavior: public void testDecode8() { final byte[] buf = new byte[] { 'H', 'e', 'l', 'l', 'o', ',', 0, 'W', 'o', 'r', 'l', 'd'} ; ByteBuffer src = ByteBuffer.wrap(buf); CharsetDecoder dec = Charset.forName("ISO-8859-1").newDecoder(); CharBuffer greeting = CharBuffer.allocate(20); CoderResult stat = dec.decode(src, greeting, true); assertSame(CoderResult.UNDERFLOW, stat); stat = dec.decode(src, greeting, true); assertSame(CoderResult.UNDERFLOW, stat); assertEquals(12, src.position()); assertEquals("Hello,\u0000World", greeting.flip().toString()); } If appears that if a null byte is encountered in the source, a null character is decoded. It does not return an error condition like the patch intended to do. The patch adds a C++ equivalent of the previous test and modified the MbstowcsCharsetDecoder::decode implementation to avoid calling mbstowcs when the string starts with a null character. It is possible that the same weakness exists in the other decoders, hopefully the unit test will catch those and they can be corrected.

          People

          • Assignee:
            Curt Arnold
            Reporter:
            hunger
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development