kicad-developers team mailing list archive
-
kicad-developers team
-
Mailing list archive
-
Message #16860
Re: [PATCH] vrml_v2_modelparser coverity fix + check
Hah, noticed I forgot to subtract a byte for the first byte read by default.
Use attached patches instead.
From 410c30db3a435c023fa93e9e9e38166263fd8287 Mon Sep 17 00:00:00 2001
From: Mark Roszko <mark.roszko@xxxxxxxxx>
Date: Tue, 17 Feb 2015 20:33:01 -0500
Subject: [PATCH 1/2] Fix vrml_v2_modelparser.cpp array comparisons
---
3d-viewer/vrml_v2_modelparser.cpp | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/3d-viewer/vrml_v2_modelparser.cpp b/3d-viewer/vrml_v2_modelparser.cpp
index 7cb9c3f..4d8618f 100644
--- a/3d-viewer/vrml_v2_modelparser.cpp
+++ b/3d-viewer/vrml_v2_modelparser.cpp
@@ -262,10 +262,10 @@ int VRML2_MODEL_PARSER::read_DEF_Coordinate()
while( GetNextTag( m_file, text ) )
{
- if( ( text == NULL ) || ( *text == ']' ) )
+ if( *text == ']' )
continue;
- if( ( *text == '}' ) )
+ if( *text == '}' )
return 0;
if( strcmp( text, "Coordinate" ) == 0 )
@@ -669,10 +669,10 @@ int VRML2_MODEL_PARSER::read_IndexedLineSet()
while( GetNextTag( m_file, text ) )
{
- if( ( text == NULL ) || ( *text == ']' ) )
+ if( *text == ']' )
continue;
- if( ( *text == '}' ) )
+ if( *text == '}' )
return 0;
if( strcmp( text, "Coordinate" ) == 0 )
@@ -874,10 +874,10 @@ int VRML2_MODEL_PARSER::read_CoordinateDef()
while( GetNextTag( m_file, text ) )
{
- if( ( text == NULL ) || ( *text == ']' ) )
+ if( *text == ']' )
continue;
- if( ( *text == '}' ) )
+ if( *text == '}' )
return 0;
if( strcmp( text, "point" ) == 0 )
--
1.9.1
From 1c58e335c9c160c6b32c998b4b7608e4ad9c3dd5 Mon Sep 17 00:00:00 2001
From: Mark Roszko <mark.roszko@xxxxxxxxx>
Date: Tue, 17 Feb 2015 20:52:59 -0500
Subject: [PATCH 2/2] Make GetNextTag implementation safer by checking number
of bytes read, also use bool return type as the function was used as such in
all cases
---
3d-viewer/vrml_aux.cpp | 10 +++++----
3d-viewer/vrml_aux.h | 2 +-
3d-viewer/vrml_v1_modelparser.cpp | 10 ++++-----
3d-viewer/vrml_v2_modelparser.cpp | 44 +++++++++++++++++++--------------------
4 files changed, 34 insertions(+), 32 deletions(-)
diff --git a/3d-viewer/vrml_aux.cpp b/3d-viewer/vrml_aux.cpp
index 8422efa..7611c4d 100644
--- a/3d-viewer/vrml_aux.cpp
+++ b/3d-viewer/vrml_aux.cpp
@@ -94,13 +94,13 @@ static int SkipGetChar( FILE* File )
}
-char* GetNextTag( FILE* File, char* tag )
+bool GetNextTag( FILE* File, char* tag, size_t len )
{
int c = SkipGetChar( File );
if( c == EOF )
{
- return NULL;
+ return false;
}
tag[0] = c;
@@ -109,9 +109,10 @@ char* GetNextTag( FILE* File, char* tag )
// DBG( printf( "tag[0] %c\n", tag[0] ) );
if( (c != '}') && (c != ']') )
{
+ len--;
char* dst = &tag[1];
- while( fscanf( File, "%c", dst ) )
+ while( fscanf( File, "%c", dst ) && len > 0 )
{
if( (*dst == ' ') || (*dst == '[') || (*dst == '{')
|| (*dst == '\t') || (*dst == '\n')|| (*dst == '\r') )
@@ -121,6 +122,7 @@ char* GetNextTag( FILE* File, char* tag )
}
dst++;
+ len--;
}
@@ -134,7 +136,7 @@ char* GetNextTag( FILE* File, char* tag )
}
}
- return tag;
+ return true;
}
diff --git a/3d-viewer/vrml_aux.h b/3d-viewer/vrml_aux.h
index f245ce7..718ad72 100644
--- a/3d-viewer/vrml_aux.h
+++ b/3d-viewer/vrml_aux.h
@@ -52,6 +52,6 @@ int read_NotImplemented( FILE* File, char closeChar);
int parseVertexList( FILE* File, std::vector< glm::vec3 > &dst_vector);
int parseVertex( FILE* File, glm::vec3 &dst_vertex );
int parseFloat( FILE* File, float *dst_float );
-char* GetNextTag( FILE* File, char* tag );
+bool GetNextTag( FILE* File, char* tag, size_t len );
#endif
diff --git a/3d-viewer/vrml_v1_modelparser.cpp b/3d-viewer/vrml_v1_modelparser.cpp
index ee7a2fd..97cc69b 100644
--- a/3d-viewer/vrml_v1_modelparser.cpp
+++ b/3d-viewer/vrml_v1_modelparser.cpp
@@ -88,7 +88,7 @@ void VRML1_MODEL_PARSER::Load( const wxString& aFilename, double aVrmlunits_to_3
childs.clear();
- while( GetNextTag( m_file, text ) )
+ while( GetNextTag( m_file, text, sizeof(text) ) )
{
if( ( *text == '}' ) || ( *text == ']' ) )
{
@@ -123,7 +123,7 @@ int VRML1_MODEL_PARSER::read_separator()
// DBG( printf( "Separator\n" ) );
- while( GetNextTag( m_file, text ) )
+ while( GetNextTag( m_file, text, sizeof(text) ) )
{
if( strcmp( text, "Material" ) == 0 )
{
@@ -182,7 +182,7 @@ int VRML1_MODEL_PARSER::readMaterial()
m_model->m_Materials = material;
- while( GetNextTag( m_file, text ) )
+ while( GetNextTag( m_file, text, sizeof(text) ) )
{
if( *text == ']' )
{
@@ -230,7 +230,7 @@ int VRML1_MODEL_PARSER::readCoordinate3()
// DBG( printf( " readCoordinate3\n" ) );
- while( GetNextTag( m_file, text ) )
+ while( GetNextTag( m_file, text, sizeof(text) ) )
{
if( *text == ']' )
{
@@ -258,7 +258,7 @@ int VRML1_MODEL_PARSER::readIndexedFaceSet()
// DBG( printf( " readIndexedFaceSet\n" ) );
- while( GetNextTag( m_file, text ) )
+ while( GetNextTag( m_file, text, sizeof(text) ) )
{
if( *text == ']' )
{
diff --git a/3d-viewer/vrml_v2_modelparser.cpp b/3d-viewer/vrml_v2_modelparser.cpp
index 4d8618f..97ba12c 100644
--- a/3d-viewer/vrml_v2_modelparser.cpp
+++ b/3d-viewer/vrml_v2_modelparser.cpp
@@ -99,7 +99,7 @@ void VRML2_MODEL_PARSER::Load( const wxString& aFilename, double aVrmlunits_to_3
childs.clear();
- while( GetNextTag( m_file, text ) )
+ while( GetNextTag( m_file, text, sizeof(text) ) )
{
if( ( *text == '}' ) || ( *text == ']' ) )
{
@@ -138,7 +138,7 @@ int VRML2_MODEL_PARSER::read_Transform()
{
char text[128];
- while( GetNextTag( m_file, text ) )
+ while( GetNextTag( m_file, text, sizeof(text) ) )
{
if( *text == ']' )
{
@@ -257,10 +257,10 @@ int VRML2_MODEL_PARSER::read_DEF_Coordinate()
char text[128];
// Get the name of the definition.
- GetNextTag( m_file, text );
+ GetNextTag( m_file, text, sizeof(text) );
std::string coordinateName = text;
- while( GetNextTag( m_file, text ) )
+ while( GetNextTag( m_file, text, sizeof(text) ) )
{
if( *text == ']' )
continue;
@@ -287,9 +287,9 @@ int VRML2_MODEL_PARSER::read_DEF()
{
char text[128];
- GetNextTag( m_file, text );
+ GetNextTag( m_file, text, sizeof(text) );
- while( GetNextTag( m_file, text ) )
+ while( GetNextTag( m_file, text, sizeof(text) ) )
{
if( *text == ']' )
{
@@ -344,7 +344,7 @@ int VRML2_MODEL_PARSER::read_USE()
char text[128];
// Get the name of the definition.
- GetNextTag( m_file, text );
+ GetNextTag( m_file, text, sizeof(text) );
std::string coordinateName = text;
// Look for it in our coordinate map.
@@ -368,7 +368,7 @@ int VRML2_MODEL_PARSER::read_Shape()
{
char text[128];
- while( GetNextTag( m_file, text ) )
+ while( GetNextTag( m_file, text, sizeof(text) ) )
{
if( *text == ']' )
{
@@ -418,7 +418,7 @@ int VRML2_MODEL_PARSER::read_Appearance()
{
char text[128];
- while( GetNextTag( m_file, text ) )
+ while( GetNextTag( m_file, text, sizeof(text) ) )
{
if( *text == ']' )
{
@@ -446,7 +446,7 @@ int VRML2_MODEL_PARSER::read_material()
S3D_MATERIAL* material = NULL;
char text[128];
- if( GetNextTag( m_file, text ) )
+ if( GetNextTag( m_file, text, sizeof(text) ) )
{
if( strcmp( text, "Material" ) == 0 )
{
@@ -462,7 +462,7 @@ int VRML2_MODEL_PARSER::read_material()
}
else if( strcmp( text, "DEF" ) == 0 )
{
- if( GetNextTag( m_file, text ) )
+ if( GetNextTag( m_file, text, sizeof(text) ) )
{
wxString mat_name;
mat_name = FROM_UTF8( text );
@@ -471,7 +471,7 @@ int VRML2_MODEL_PARSER::read_material()
GetMaster()->Insert( material );
m_model->m_Materials = material;
- if( GetNextTag( m_file, text ) )
+ if( GetNextTag( m_file, text, sizeof(text) ) )
{
if( strcmp( text, "Material" ) == 0 )
{
@@ -482,7 +482,7 @@ int VRML2_MODEL_PARSER::read_material()
}
else if( strcmp( text, "USE" ) == 0 )
{
- if( GetNextTag( m_file, text ) )
+ if( GetNextTag( m_file, text, sizeof(text) ) )
{
wxString mat_name;
mat_name = FROM_UTF8( text );
@@ -511,7 +511,7 @@ int VRML2_MODEL_PARSER::read_Material()
char text[128];
glm::vec3 vertex;
- while( GetNextTag( m_file, text ) )
+ while( GetNextTag( m_file, text, sizeof(text) ) )
{
if( *text == ']' )
{
@@ -593,7 +593,7 @@ int VRML2_MODEL_PARSER::read_IndexedFaceSet()
m_normalPerVertex = false;
colorPerVertex = false;
- while( GetNextTag( m_file, text ) )
+ while( GetNextTag( m_file, text, sizeof(text) ) )
{
if( *text == ']' )
{
@@ -607,7 +607,7 @@ int VRML2_MODEL_PARSER::read_IndexedFaceSet()
if( strcmp( text, "normalPerVertex" ) == 0 )
{
- if( GetNextTag( m_file, text ) )
+ if( GetNextTag( m_file, text, sizeof(text) ) )
{
if( strcmp( text, "TRUE" ) == 0 )
{
@@ -617,7 +617,7 @@ int VRML2_MODEL_PARSER::read_IndexedFaceSet()
}
else if( strcmp( text, "colorPerVertex" ) == 0 )
{
- GetNextTag( m_file, text );
+ GetNextTag( m_file, text, sizeof(text) );
if( strcmp( text, "TRUE" ) )
{
@@ -667,7 +667,7 @@ int VRML2_MODEL_PARSER::read_IndexedLineSet()
{
char text[128];
- while( GetNextTag( m_file, text ) )
+ while( GetNextTag( m_file, text, sizeof(text) ) )
{
if( *text == ']' )
continue;
@@ -783,7 +783,7 @@ int VRML2_MODEL_PARSER::read_Color()
{
char text[128];
- while( GetNextTag( m_file, text ) )
+ while( GetNextTag( m_file, text, sizeof(text) ) )
{
if( *text == ']' )
{
@@ -810,7 +810,7 @@ int VRML2_MODEL_PARSER::read_Normal()
{
char text[128];
- while( GetNextTag( m_file, text ) )
+ while( GetNextTag( m_file, text, sizeof(text) ) )
{
if( *text == ']' )
{
@@ -843,7 +843,7 @@ int VRML2_MODEL_PARSER::read_Coordinate()
{
char text[128];
- while( GetNextTag( m_file, text ) )
+ while( GetNextTag( m_file, text, sizeof(text) ) )
{
if( *text == ']' )
{
@@ -872,7 +872,7 @@ int VRML2_MODEL_PARSER::read_CoordinateDef()
{
char text[128];
- while( GetNextTag( m_file, text ) )
+ while( GetNextTag( m_file, text, sizeof(text) ) )
{
if( *text == ']' )
continue;
--
1.9.1
References
