diff --git a/modules/core/src/persistence.cpp b/modules/core/src/persistence.cpp index 0c516a4e8a..ae6fd3a423 100644 --- a/modules/core/src/persistence.cpp +++ b/modules/core/src/persistence.cpp @@ -59,16 +59,33 @@ char* icvGets( CvFileStorage* fs, char* str, int maxCount ) } str[j++] = '\0'; fs->strbufpos = i; + if (maxCount > 256 && !(fs->flags & cv::FileStorage::BASE64)) + CV_Assert(j < maxCount - 1 && "OpenCV persistence doesn't support very long lines"); return j > 1 ? str : 0; } if( fs->file ) - return fgets( str, maxCount, fs->file ); + { + char* ptr = fgets( str, maxCount, fs->file ); + if (ptr && maxCount > 256 && !(fs->flags & cv::FileStorage::BASE64)) + { + size_t sz = strnlen(ptr, maxCount); + CV_Assert(sz < (size_t)(maxCount - 1) && "OpenCV persistence doesn't support very long lines"); + } + return ptr; + } #if USE_ZLIB if( fs->gzfile ) - return gzgets( fs->gzfile, str, maxCount ); + { + char* ptr = gzgets( fs->gzfile, str, maxCount ); + if (ptr && maxCount > 256 && !(fs->flags & cv::FileStorage::BASE64)) + { + size_t sz = strnlen(ptr, maxCount); + CV_Assert(sz < (size_t)(maxCount - 1) && "OpenCV persistence doesn't support very long lines"); + } + return ptr; + } #endif - CV_Error( CV_StsError, "The storage is not opened" ); - return 0; + CV_ErrorNoReturn(CV_StsError, "The storage is not opened"); } int icvEof( CvFileStorage* fs ) diff --git a/modules/core/src/persistence.hpp b/modules/core/src/persistence.hpp index 392143de2b..4d045066f2 100644 --- a/modules/core/src/persistence.hpp +++ b/modules/core/src/persistence.hpp @@ -321,4 +321,8 @@ void icvJSONWriteReal( CvFileStorage* fs, const char* key, double value ); void icvJSONWriteString( CvFileStorage* fs, const char* key, const char* str, int quote CV_DEFAULT(0)); void icvJSONWriteComment( CvFileStorage* fs, const char* comment, int eol_comment ); +// Adding icvGets is not enough - we need to merge buffer contents (see #11061) +#define CV_PERSISTENCE_CHECK_END_OF_BUFFER_BUG() \ + CV_Assert((ptr[0] != 0 || ptr != fs->buffer_end - 1) && "OpenCV persistence doesn't support very long lines") + #endif // SRC_PERSISTENCE_HPP diff --git a/modules/core/src/persistence_json.cpp b/modules/core/src/persistence_json.cpp index 5d8a5bdb94..ef0c3a49f5 100644 --- a/modules/core/src/persistence_json.cpp +++ b/modules/core/src/persistence_json.cpp @@ -125,8 +125,10 @@ static char* icvJSONParseKey( CvFileStorage* fs, char* ptr, CvFileNode* map, CvF char * beg = ptr + 1; char * end = beg; - do ++ptr; - while( cv_isprint(*ptr) && *ptr != '"' ); + do { + ++ptr; + CV_PERSISTENCE_CHECK_END_OF_BUFFER_BUG(); + } while( cv_isprint(*ptr) && *ptr != '"' ); if( *ptr != '"' ) CV_PARSE_ERROR( "Key must end with \'\"\'" ); @@ -367,17 +369,25 @@ static char* icvJSONParseValue( CvFileStorage* fs, char* ptr, CvFileNode* node ) { /**************** number ****************/ char * beg = ptr; if ( *ptr == '+' || *ptr == '-' ) + { ptr++; + CV_PERSISTENCE_CHECK_END_OF_BUFFER_BUG(); + } while( cv_isdigit(*ptr) ) + { ptr++; + CV_PERSISTENCE_CHECK_END_OF_BUFFER_BUG(); + } if (*ptr == '.' || *ptr == 'e') { node->data.f = icv_strtod( fs, beg, &ptr ); + CV_PERSISTENCE_CHECK_END_OF_BUFFER_BUG(); node->tag = CV_NODE_REAL; } else { node->data.i = static_cast(strtol( beg, &ptr, 0 )); + CV_PERSISTENCE_CHECK_END_OF_BUFFER_BUG(); node->tag = CV_NODE_INT; } @@ -388,8 +398,12 @@ static char* icvJSONParseValue( CvFileStorage* fs, char* ptr, CvFileNode* node ) { /**************** other data ****************/ const char * beg = ptr; size_t len = 0u; - for ( ; cv_isalpha(*ptr) && len <= 6u; ptr++ ) + for ( ; cv_isalpha(*ptr) && len <= 6u; ) + { len++; + ptr++; + CV_PERSISTENCE_CHECK_END_OF_BUFFER_BUG(); + } if ( len >= 4u && memcmp( beg, "null", 4u ) == 0 ) { diff --git a/modules/core/src/persistence_xml.cpp b/modules/core/src/persistence_xml.cpp index 3e91881b18..8928a8354b 100644 --- a/modules/core/src/persistence_xml.cpp +++ b/modules/core/src/persistence_xml.cpp @@ -78,7 +78,7 @@ icvXMLSkipSpaces( CvFileStorage* fs, char* ptr, int mode ) ptr = icvGets( fs, fs->buffer_start, max_size ); if( !ptr ) { - ptr = fs->buffer_start; + ptr = fs->buffer_start; // FIXIT Why do we need this hack? What is about other parsers JSON/YAML? *ptr = '\0'; fs->dummy_eof = 1; break; @@ -89,7 +89,7 @@ icvXMLSkipSpaces( CvFileStorage* fs, char* ptr, int mode ) if( ptr[l-1] != '\n' && ptr[l-1] != '\r' && !icvEof(fs) ) CV_PARSE_ERROR( "Too long string or a last string w/o newline" ); } - fs->lineno++; + fs->lineno++; // FIXIT doesn't really work with long lines. It must be counted via '\n' or '\r' symbols, not the number of icvGets() calls. } } return ptr; @@ -209,14 +209,14 @@ icvXMLParseValue( CvFileStorage* fs, char* ptr, CvFileNode* node, char c = *ptr, d; char* endptr; - if( cv_isspace(c) || c == '\0' || (c == '<' && ptr[1] == '!' && ptr[2] == '-') ) + if( cv_isspace(c) || c == '\0' || (c == '<' && ptr[1] == '!' && ptr[2] == '-') ) // FIXIT ptr[1], ptr[2] - out of bounds read without check or data fetch (#11061) { ptr = icvXMLSkipSpaces( fs, ptr, 0 ); have_space = true; c = *ptr; } - d = ptr[1]; + d = ptr[1]; // FIXIT ptr[1] - out of bounds read without check or data fetch (#11061) if( c =='<' || c == '\0' ) { @@ -238,7 +238,7 @@ icvXMLParseValue( CvFileStorage* fs, char* ptr, CvFileNode* node, if( tag_type == CV_XML_EMPTY_TAG ) CV_PARSE_ERROR( "Empty tags are not supported" ); - assert( tag_type == CV_XML_OPENING_TAG ); + CV_Assert(tag_type == CV_XML_OPENING_TAG); /* for base64 string */ bool is_binary_string = false; @@ -339,6 +339,7 @@ icvXMLParseValue( CvFileStorage* fs, char* ptr, CvFileNode* node, CV_PARSE_ERROR( "Invalid numeric value (inconsistent explicit type specification?)" ); ptr = endptr; + CV_PERSISTENCE_CHECK_END_OF_BUFFER_BUG(); } else { @@ -354,6 +355,7 @@ icvXMLParseValue( CvFileStorage* fs, char* ptr, CvFileNode* node, for( ;; ) { c = *++ptr; + CV_PERSISTENCE_CHECK_END_OF_BUFFER_BUG(); if( !cv_isalnum(c) ) { if( c == '\"' ) @@ -415,6 +417,7 @@ icvXMLParseValue( CvFileStorage* fs, char* ptr, CvFileNode* node, } } ptr = endptr; + CV_PERSISTENCE_CHECK_END_OF_BUFFER_BUG(); } } buf[i++] = c; @@ -471,6 +474,7 @@ icvXMLParseTag( CvFileStorage* fs, char* ptr, CvStringHashNode** _tag, CV_PARSE_ERROR( "Tag should start with \'<\'" ); ptr++; + CV_PERSISTENCE_CHECK_END_OF_BUFFER_BUG(); if( cv_isalnum(*ptr) || *ptr == '_' ) tag_type = CV_XML_OPENING_TAG; else if( *ptr == '/' ) @@ -506,6 +510,7 @@ icvXMLParseTag( CvFileStorage* fs, char* ptr, CvStringHashNode** _tag, attrname = cvGetHashedKey( fs, ptr, (int)(endptr - ptr), 1 ); CV_Assert(attrname); ptr = endptr; + CV_PERSISTENCE_CHECK_END_OF_BUFFER_BUG(); if( !tagname ) tagname = attrname; @@ -573,12 +578,12 @@ icvXMLParseTag( CvFileStorage* fs, char* ptr, CvStringHashNode** _tag, } else if( c == '?' && tag_type == CV_XML_HEADER_TAG ) { - if( ptr[1] != '>' ) + if( ptr[1] != '>' ) // FIXIT ptr[1] - out of bounds read without check CV_PARSE_ERROR( "Invalid closing tag for ' && tag_type == CV_XML_OPENING_TAG ) + else if( c == '/' && ptr[1] == '>' && tag_type == CV_XML_OPENING_TAG ) // FIXIT ptr[1] - out of bounds read without check { tag_type = CV_XML_EMPTY_TAG; ptr += 2; @@ -607,7 +612,7 @@ void icvXMLParse( CvFileStorage* fs ) // CV_XML_INSIDE_TAG is used to prohibit leading comments ptr = icvXMLSkipSpaces( fs, ptr, CV_XML_INSIDE_TAG ); - if( memcmp( ptr, "\'" ); ptr = icvXMLParseTag( fs, ptr, &key, &list, &tag_type ); @@ -648,8 +653,7 @@ void icvXMLParse( CvFileStorage* fs ) ptr = icvXMLSkipSpaces( fs, ptr, 0 ); } } - - assert( fs->dummy_eof != 0 ); + CV_Assert( fs->dummy_eof != 0 ); } diff --git a/modules/core/src/persistence_yml.cpp b/modules/core/src/persistence_yml.cpp index eba00a31c7..ab6ebf6eda 100644 --- a/modules/core/src/persistence_yml.cpp +++ b/modules/core/src/persistence_yml.cpp @@ -51,7 +51,7 @@ static char* icvYMLSkipSpaces( CvFileStorage* fs, char* ptr, int min_indent, int CV_PARSE_ERROR( "Too long string or a last string w/o newline" ); } - fs->lineno++; + fs->lineno++; // FIXIT doesn't really work with long lines. It must be counted via '\n' or '\r' symbols, not the number of icvGets() calls. } else CV_PARSE_ERROR( *ptr == '\t' ? "Tabs are prohibited in YAML!" : "Invalid character" ); @@ -331,6 +331,7 @@ force_int: CV_PARSE_ERROR( "Invalid numeric value (inconsistent explicit type specification?)" ); ptr = endptr; + CV_PERSISTENCE_CHECK_END_OF_BUFFER_BUG(); } else if( c == '\'' || c == '\"' ) // an explicit string {