ERROR: AddressSanitizer: global-buffer-overflow in function ErrorIDToName

[thientc]

Instrument Futag found this error with tinyxml2 version 9.0.0 and in current version.

tinyxml2/tinyxml2.cpp

Lines 2501 to 2507 in e45d9d1

	/*static*/ const char* XMLDocument::ErrorIDToName(XMLError errorID)
	{
	TIXMLASSERT( errorID >= 0 && errorID < XML_ERROR_COUNT );
	const char* errorName = _errorNames[errorID];
	TIXMLASSERT( errorName && errorName[0] );
	return errorName;
	}

errorID is a variable of XMLError type, which can receive value from XML_SUCCESS (0) to XML_ERROR_COUNT (19)

tinyxml2/tinyxml2.h

Lines 523 to 545 in e45d9d1

	enum XMLError {
	XML_SUCCESS = 0,
	XML_NO_ATTRIBUTE,
	XML_WRONG_ATTRIBUTE_TYPE,
	XML_ERROR_FILE_NOT_FOUND,
	XML_ERROR_FILE_COULD_NOT_BE_OPENED,
	XML_ERROR_FILE_READ_ERROR,
	XML_ERROR_PARSING_ELEMENT,
	XML_ERROR_PARSING_ATTRIBUTE,
	XML_ERROR_PARSING_TEXT,
	XML_ERROR_PARSING_CDATA,
	XML_ERROR_PARSING_COMMENT,
	XML_ERROR_PARSING_DECLARATION,
	XML_ERROR_PARSING_UNKNOWN,
	XML_ERROR_EMPTY_DOCUMENT,
	XML_ERROR_MISMATCHED_ELEMENT,
	XML_ERROR_PARSING,
	XML_CAN_NOT_CONVERT_TEXT,
	XML_NO_TEXT_NODE,
	XML_ELEMENT_DEPTH_EXCEEDED,
	XML_ERROR_COUNT
	};

The _errorNames array has 19 elements (from 0 to 18) and was defined here:

tinyxml2/tinyxml2.cpp

Lines 2136 to 2156 in e45d9d1

	const char* XMLDocument::_errorNames[XML_ERROR_COUNT] = {
	"XML_SUCCESS",
	"XML_NO_ATTRIBUTE",
	"XML_WRONG_ATTRIBUTE_TYPE",
	"XML_ERROR_FILE_NOT_FOUND",
	"XML_ERROR_FILE_COULD_NOT_BE_OPENED",
	"XML_ERROR_FILE_READ_ERROR",
	"XML_ERROR_PARSING_ELEMENT",
	"XML_ERROR_PARSING_ATTRIBUTE",
	"XML_ERROR_PARSING_TEXT",
	"XML_ERROR_PARSING_CDATA",
	"XML_ERROR_PARSING_COMMENT",
	"XML_ERROR_PARSING_DECLARATION",
	"XML_ERROR_PARSING_UNKNOWN",
	"XML_ERROR_EMPTY_DOCUMENT",
	"XML_ERROR_MISMATCHED_ELEMENT",
	"XML_ERROR_PARSING",
	"XML_CAN_NOT_CONVERT_TEXT",
	"XML_NO_TEXT_NODE",
	"XML_ELEMENT_DEPTH_EXCEEDED"
	};

So, when errorID gets XML_ERROR_COUNT value, error occurs at instruction const char* errorName = _errorNames[errorID];

The generated fuzzing wrapper is attached below.
ErrorIDToName1.cpp.zip

[LamboPita]

TIXMLASSERT( errorID >= 0 && errorID < XML_ERROR_COUNT );

Looks like a false positive. XML_ERROR_COUNT is a placeholder used as a constant size and not an actual XMLError.

[thientc]

TIXMLASSERT( errorID >= 0 && errorID < XML_ERROR_COUNT );

Looks like a false positive. XML_ERROR_COUNT is a placeholder used as a constant size and not an actual XMLError.

Is it positive If the TIXMLASSERT does not work on line 95 (commit 1dee28e - version 9.0.0)?

tinyxml2/tinyxml2.h

Lines 82 to 97 in 1dee28e

	#if !defined(TIXMLASSERT)
	#if defined(TINYXML2_DEBUG)
	# if defined(_MSC_VER)
	# // "(void)0," is for suppressing C4127 warning in "assert(false)", "assert(true)" and the like
	# define TIXMLASSERT( x ) if ( !((void)0,(x))) { __debugbreak(); }
	# elif defined (ANDROID_NDK)
	# include <android/log.h>
	# define TIXMLASSERT( x ) if ( !(x)) { __android_log_assert( "assert", "grinliz", "ASSERT in '%s' at %d.", __FILE__, __LINE__ ); }
	# else
	# include <assert.h>
	# define TIXMLASSERT assert
	# endif
	#else
	# define TIXMLASSERT( x ) {}
	#endif
	#endif

This line has been fixed with current commit

tinyxml2/tinyxml2.h

Line 95 in e45d9d1

# define TIXMLASSERT( x ) do {} while(false)