← Back to team overview

kicad-developers team mailing list archive

[PATCH] Fix buffer overflows in eeschema

 

Eeschema is _full_ of sscanf buffer overflow vulnerabilities, in almost 
every ::Load. This patch adds the proper field width specifiers to 
prevent the buffers from being smashed by an invalid or malicious input.

--
Chris

commit 1cc7418975a7391ba61e480525ee12ae229264cc
Author: Chris Pavlina <cpavlin1@xxxxxxxxxxxxxx>
Date:   Thu Jun 25 00:35:23 2015 -0400

    Add field width specifiers to sscanf

diff --git a/eeschema/lib_circle.cpp b/eeschema/lib_circle.cpp
index 646834e..bcf6a40 100644
--- a/eeschema/lib_circle.cpp
+++ b/eeschema/lib_circle.cpp
@@ -68,7 +68,7 @@ bool LIB_CIRCLE::Load( LINE_READER& aLineReader, wxString& aErrorMsg )
     char tmp[256];
     char* line = (char*) aLineReader;
 
-    int  cnt = sscanf( line + 2, "%d %d %d %d %d %d %s", &m_Pos.x, &m_Pos.y,
+    int  cnt = sscanf( line + 2, "%d %d %d %d %d %d %255s", &m_Pos.x, &m_Pos.y,
                        &m_Radius, &m_Unit, &m_Convert, &m_Width, tmp );
 
     if( cnt < 6 )
diff --git a/eeschema/lib_field.cpp b/eeschema/lib_field.cpp
index 914d38a..a223b34 100644
--- a/eeschema/lib_field.cpp
+++ b/eeschema/lib_field.cpp
@@ -160,7 +160,7 @@ bool LIB_FIELD::Load( LINE_READER& aLineReader, wxString& errorMsg )
 
     memset( textVJustify, 0, sizeof( textVJustify ) );
 
-    cnt = sscanf( line, " %d %d %d %c %c %c %s", &m_Pos.x, &m_Pos.y, &m_Size.y,
+    cnt = sscanf( line, " %d %d %d %c %c %c %255s", &m_Pos.x, &m_Pos.y, &m_Size.y,
                   &textOrient, &textVisible, &textHJustify, textVJustify );
 
     if( cnt < 5 )
diff --git a/eeschema/lib_pin.cpp b/eeschema/lib_pin.cpp
index c4dd914..7496009 100644
--- a/eeschema/lib_pin.cpp
+++ b/eeschema/lib_pin.cpp
@@ -730,7 +730,7 @@ bool LIB_PIN::Load( LINE_READER& aLineReader, wxString& aErrorMsg )
 
     *pinAttrs = 0;
 
-    i = sscanf( line + 2, "%s %s %d %d %d %s %d %d %d %d %s %s", pinName,
+    i = sscanf( line + 2, "%255s %63s %d %d %d %63s %d %d %d %d %63s %63s", pinName,
                 pinNum, &m_position.x, &m_position.y, &m_length, pinOrient, &m_numTextSize,
                 &m_nameTextSize, &m_Unit, &m_Convert, pinType, pinAttrs );
 
diff --git a/eeschema/load_one_schematic_file.cpp b/eeschema/load_one_schematic_file.cpp
index b1aeb0a..4afc8fd 100644
--- a/eeschema/load_one_schematic_file.cpp
+++ b/eeschema/load_one_schematic_file.cpp
@@ -217,7 +217,7 @@ again." );
             break;
 
         case 'T':                       // It is a text item.
-            if( sscanf( sline, "%s", name1 ) != 1 )
+            if( sscanf( sline, "%255s", name1 ) != 1 )
             {
                 msgDiag.Printf( _( "Eeschema file text load error at line %d" ),
                                 reader.LineNumber() );
diff --git a/eeschema/sch_bus_entry.cpp b/eeschema/sch_bus_entry.cpp
index 9f73d73..52c37b7 100644
--- a/eeschema/sch_bus_entry.cpp
+++ b/eeschema/sch_bus_entry.cpp
@@ -123,7 +123,7 @@ bool SCH_BUS_ENTRY_BASE::Load( LINE_READER& aLine, wxString& aErrorMsg,
     while( (*line != ' ' ) && *line )
         line++;
 
-    if( sscanf( line, "%s %s", Name1, Name2 ) != 2  )
+    if( sscanf( line, "%255s %255s", Name1, Name2 ) != 2  )
     {
         aErrorMsg.Printf( wxT( "Eeschema file bus entry load error at line %d" ),
                           aLine.LineNumber() );
diff --git a/eeschema/sch_component.cpp b/eeschema/sch_component.cpp
index 6a3f639..ddf2f14 100644
--- a/eeschema/sch_component.cpp
+++ b/eeschema/sch_component.cpp
@@ -1148,7 +1148,7 @@ bool SCH_COMPONENT::Load( LINE_READER& aLine, wxString& aErrorMsg )
             return true;
     }
 
-    if( sscanf( &line[1], "%s %s", name1, name2 ) != 2 )
+    if( sscanf( &line[1], "%255s %255s", name1, name2 ) != 2 )
     {
         aErrorMsg.Printf( wxT( "Eeschema component description error at line %d, aborted" ),
                           aLine.LineNumber() );
@@ -1354,7 +1354,7 @@ bool SCH_COMPONENT::Load( LINE_READER& aLine, wxString& aErrorMsg )
             memset( char3, 0, sizeof(char3) );
             int x, y, w, attr;
 
-            if( ( ii = sscanf( ptcar, "%s %d %d %d %X %s %s", char1, &x, &y, &w, &attr,
+            if( ( ii = sscanf( ptcar, "%255s %d %d %d %X %255s %255s", char1, &x, &y, &w, &attr,
                                char2, char3 ) ) < 4 )
             {
                 aErrorMsg.Printf( wxT( "Component Field error line %d, aborted" ),
diff --git a/eeschema/sch_junction.cpp b/eeschema/sch_junction.cpp
index 44e6a03..a12f21b 100644
--- a/eeschema/sch_junction.cpp
+++ b/eeschema/sch_junction.cpp
@@ -86,7 +86,7 @@ bool SCH_JUNCTION::Load( LINE_READER& aLine, wxString& aErrorMsg )
     while( (*line != ' ' ) && *line )
         line++;
 
-    if( sscanf( line, "%s %d %d", name, &m_pos.x, &m_pos.y ) != 3 )
+    if( sscanf( line, "%255s %d %d", name, &m_pos.x, &m_pos.y ) != 3 )
     {
         aErrorMsg.Printf( wxT( "Eeschema file connection load error at line %d, aborted" ),
                           aLine.LineNumber() );
diff --git a/eeschema/sch_line.cpp b/eeschema/sch_line.cpp
index a2b5716..a786332 100644
--- a/eeschema/sch_line.cpp
+++ b/eeschema/sch_line.cpp
@@ -174,7 +174,7 @@ bool SCH_LINE::Load( LINE_READER& aLine, wxString& aErrorMsg )
     while( (*line != ' ' ) && *line )
         line++;
 
-    if( sscanf( line, "%s %s", Name1, Name2 ) != 2  )
+    if( sscanf( line, "%255s %255s", Name1, Name2 ) != 2  )
     {
         aErrorMsg.Printf( wxT( "Eeschema file segment error at line %d, aborted" ),
                           aLine.LineNumber() );
diff --git a/eeschema/sch_no_connect.cpp b/eeschema/sch_no_connect.cpp
index 728bf48..694069d 100644
--- a/eeschema/sch_no_connect.cpp
+++ b/eeschema/sch_no_connect.cpp
@@ -101,7 +101,7 @@ bool SCH_NO_CONNECT::Load( LINE_READER& aLine, wxString& aErrorMsg )
     while( (*line != ' ' ) && *line )
         line++;
 
-    if( sscanf( line, "%s %d %d", name, &m_pos.x, &m_pos.y ) != 3 )
+    if( sscanf( line, "%255s %d %d", name, &m_pos.x, &m_pos.y ) != 3 )
     {
         aErrorMsg.Printf( wxT( "Eeschema file No Connect load error at line %d" ),
                           aLine.LineNumber() );
diff --git a/eeschema/sch_text.cpp b/eeschema/sch_text.cpp
index 7422662..762ffbd 100644
--- a/eeschema/sch_text.cpp
+++ b/eeschema/sch_text.cpp
@@ -428,7 +428,7 @@ bool SCH_TEXT::Load( LINE_READER& aLine, wxString& aErrorMsg )
         sline++;
 
     // sline points the start of parameters
-    int ii = sscanf( sline, "%s %d %d %d %d %s %s %d", Name1, &m_Pos.x, &m_Pos.y,
+    int ii = sscanf( sline, "%255s %d %d %d %d %255s %255s %d", Name1, &m_Pos.x, &m_Pos.y,
                      &orient, &size, Name2, Name3, &thickness );
 
     if( ii < 4 )
@@ -906,7 +906,7 @@ bool SCH_LABEL::Load( LINE_READER& aLine, wxString& aErrorMsg )
         sline++;
 
     // sline points the start of parameters
-    int ii = sscanf( sline, "%s %d %d %d %d %s %s %d", Name1, &m_Pos.x, &m_Pos.y,
+    int ii = sscanf( sline, "%255s %d %d %d %d %255s %255s %d", Name1, &m_Pos.x, &m_Pos.y,
                      &orient, &size, Name2, Name3, &thickness );
 
     if( ii < 4 )
@@ -1067,7 +1067,7 @@ bool SCH_GLOBALLABEL::Load( LINE_READER& aLine, wxString& aErrorMsg )
         sline++;
 
     // sline points the start of parameters
-    int ii = sscanf( sline, "%s %d %d %d %d %s %s %d", Name1, &m_Pos.x, &m_Pos.y,
+    int ii = sscanf( sline, "%255s %d %d %d %d %255s %255s %d", Name1, &m_Pos.x, &m_Pos.y,
                      &orient, &size, Name2, Name3, &thickness );
 
     if( ii < 4 )
@@ -1496,7 +1496,7 @@ bool SCH_HIERLABEL::Load( LINE_READER& aLine, wxString& aErrorMsg )
         sline++;
 
     // sline points the start of parameters
-    int ii = sscanf( sline, "%s %d %d %d %d %s %s %d", Name1, &m_Pos.x, &m_Pos.y,
+    int ii = sscanf( sline, "%255s %d %d %d %d %255s %255s %d", Name1, &m_Pos.x, &m_Pos.y,
                      &orient, &size, Name2, Name3, &thickness );
 
     if( ii < 4 )

Follow ups