purify reports uninitialized memory read in _rw_get_char

• Key: STDCXX-597
• Type: Improvement
• Status: Open
• Priority: Minor
• Assignee: Unassigned
• Reporter: Travis Vitek
• Component/s: Test Driver
• Affects Version/s: 4.2.0
• Fix Version/s: 4.3.0

Description

****  Purify instrumented ./21.string.access (pid 2878)  ****
UMR: Uninitialized memory read:
  * This is occurring while in thread 2878:
    _rw_get_char(char const*, char const**, unsigned*) [char.cpp:562]
    rw_match(char const*, char const*, unsigned) [char.cpp:816]
     test_access<char, std::char_traits<char>, std::allocator<char> >(char, std::char_traits<char>*, char*, StringFunc const&, StringTestCase const&) [21.string.access.cpp:274]
    test_access(StringFunc const&, StringTestCase const&) [21.string.access.cpp:317]
    _rw_test_case(StringFunc const&, StringTestCase const&,   (*)(StringFunc const&, StringTestCase const&)) [21.strings.cpp:1298]
    _rw_run_cases(StringFunc const&, StringTest const&) [21.strings.cpp:1353]
  * Reading 1 byte from 0x8182256 in the heap.
  * Address 0x8182256 is 14 bytes into a malloc'd block at 0x8182248 of 46 bytes.
  * This block was allocated from thread -1207973632:
    malloc         [rtlib.o]
    operator new(unsigned) [libstd15d.so]
    __rw::__rw_allocate(unsigned, int) [memory.cpp:53]
    std::allocator<char>::allocate(unsigned,  const*) [_allocator.h:144]
    std::string<char, std::char_traits<char>, std::allocator<char>>::_C_get_rep(unsigned, unsigned) [string.cc:102]
    std::string<char, std::char_traits<char>, std::allocator<char>>::string<char, std::char_traits<char>, std::allocator<char>>[not-in-charge](char const*, unsigned, std::allocator<char> const&) [string.cc:180]

****  Purify instrumented ./21.string.access (pid 2878)  ****
UMR: Uninitialized memory read:
  * This is occurring while in thread 2878:
    _rw_get_char(char const*, char const**, unsigned*) [char.cpp:562]
    rw_match(char const*, char const*, unsigned) [char.cpp:816]
     test_access<char, UserTraits<char>, std::allocator<char> >(char, UserTraits<char>*, char*, StringFunc const&, StringTestCase const&) [21.string.access.cpp:274]
    test_access(StringFunc const&, StringTestCase const&) [21.string.access.cpp:317]
    _rw_test_case(StringFunc const&, StringTestCase const&,   (*)(StringFunc const&, StringTestCase const&)) [21.strings.cpp:1298]
    _rw_run_cases(StringFunc const&, StringTest const&) [21.strings.cpp:1353]
  * Reading 1 byte from 0x818d5a6 in the heap.
  * Address 0x818d5a6 is 14 bytes into a malloc'd block at 0x818d598 of 46 bytes.
  * This block was allocated from thread -1207973632:
    malloc         [rtlib.o]
    operator new(unsigned) [libstd15d.so]
    __rw::__rw_allocate(unsigned, int) [memory.cpp:53]
    std::allocator<char>::allocate(unsigned,  const*) [_allocator.h:144]
    std::basic_string<char, std::char_traits<char>, std::allocator<char>><char, UserTraits<char>, std::allocator<char> >::_C_get_rep(unsigned, unsigned) [string.cc:102]
    std::basic_string<char, std::char_traits<char>, std::allocator<char>><char, UserTraits<char>, std::allocator<char> >::basic_string<char, std::char_traits<char>, std::allocator<char>>[not-in-charge](char const*, unsigned, std::allocator<char> const&) [string.cc:180] 

Comments

2007-10-15 Travis Vitek <[e-mail]>

STDCXX-597

• char.cpp (_rw_char_get): Check character count before
  examining string contents to avoid uninitialized memory read.

[Martin Sebor]

Reported against 4.2 Set Affects Version(s) accordingly.

[Martin Sebor]

Travis, I get one assertion from 0.char before applying the patch:

# ASSERTION (S7) (3 lines):
# TEXT: rw_printf(">%{/*Gs}<", ...) == ">\"\"<"; got ">(misaligned address 0x000000000042c8a7)<"
# LINE: 1142

and 5 after applying it:

# ASSERTION (S7) (3 lines):
# TEXT: rw_match("\0@0a\0@0b", "ab", 2) == 140733193388034, got 1
# LINE: 819

# ASSERTION (S7) (3 lines):
# TEXT: rw_match("a@0a@1a@2a@3", "a@3a@2a@1a@0", 7) == 140733193388039, got 6
# LINE: 855

# ASSERTION (S7) (3 lines):
# TEXT: rw_match("\0@0a\0@0b", L"ab", 2) == 140733193388034, got 1
# LINE: 970

# ASSERTION (S7) (3 lines):
# TEXT: rw_match("\0@0a\0@0b", "ab", 2) == 140733193388034, got 1
# LINE: 1063

# ASSERTION (S7) (3 lines):
# TEXT: rw_printf(">%{/*Gs}<", ...) == ">\"\"<"; got ">(misaligned address 0x000000000042c8c7)<"
# LINE: 1142

[Martin Sebor]

Disabled formatting.

[Travis Vitek]

The test_access() function in 21.string.access.cpp uses rw_match() to verify that two empty strings are equal. It does so with a line that looks something like this...

        const bool success = 1 == rw_match (exp_res, pres, 1);

The problem is that the string `pres' only has 1 byte of initialized data, and that one byte is the terminator for the string. The rw_match() function expects that the provided `len' is the number of characters before the null terminator. So this seems like a bug in test_access(), not in _rw_get_char.

Here is a simplified testcase that shows the UMR when misusing rw_match().

#include <rw_char.h> // for rw_match()
#include <stdlib.h>     // for malloc()

int main ()
{
    char* s = (char*)malloc (10);
    *s = '\0';

    const char u [2] = {
        '\0', '\0'
    };

    rw_match (u, s, 1);

    free (s);

    return 0;
}

[Travis Vitek]

Commited to trunk in r632418.

[Travis Vitek]

Reverted on trunk in r632436. The previous change only worked around the issue by not checking the result, which would essentially disable some tests, which would be bad.

[Travis Vitek]

Inside _rw_get_char(), *count is not the length of the src string as I had originally expected. It is essentially the number of character 'tokens' that we expect the src string to have. Unfortunately, this is not enough information. We need to know the length of the src buffer so that we can avoid reading past the end of the string. The above testcase shows the problem, but you need to debug into rw_match() to see it.

This is a problem that could cause unexpected test failures. If _rw_get_char() is given a pointer to the null terminator at the end of a string, and the characters following that null terminator just happen to be '@N' where N is some positive integer value, we will get unexpected results because _rw_get_char() will think that it was given the input string "\0@N", when in all reality it was just given an empty string. The bottom line is that if your function is supposed to handle embedded nulls, you need to allow the user to provide a length for each buffer that is passed in.

[Travis Vitek]

The rw_match() and _rw_get_char() functions need to know the length of the input string to avoid reading past the end. This problem doesn't have an adverse effect on the pass rate, so I'm deferring it to 4.3.

[Farid Zaripov]

See also this thread: http://www.mail-archive.com/[e-mail]/msg01654.html