From a7fbdfe9182fe075d1f36cf1f23432b28caf03b3 Mon Sep 17 00:00:00 2001 From: Seth Hillbrand Date: Tue, 1 Feb 2022 15:49:29 -0800 Subject: [PATCH] Fix overflow vulnerability in Gerbview Corrects an unguarded read that could lead to arbitrary code execution in specifically crafted gerber files. Fixes https://gitlab.com/kicad/code/kicad/issues/10700 (cherry picked from commit 54b20cb0492ee20eb9efaff478eaa51fe17b4ca3) --- gerbview/excellon_image.h | 5 --- gerbview/excellon_read_drill_file.cpp | 2 +- gerbview/gerber_file_image.h | 9 +++-- gerbview/readgerb.cpp | 4 +-- gerbview/rs274d.cpp | 48 +++++++-------------------- 5 files changed, 21 insertions(+), 47 deletions(-) diff --git a/gerbview/excellon_image.h b/gerbview/excellon_image.h index f83c4acfa4..9919241cc2 100644 --- a/gerbview/excellon_image.h +++ b/gerbview/excellon_image.h @@ -185,11 +185,6 @@ private: */ bool readToolInformation( char*& aText ); - int TCodeNumber( char*& aText ) - { - return DCodeNumber( aText ); - } - /** * End a route command started by M15 ot G01, G02 or G03 command. */ diff --git a/gerbview/excellon_read_drill_file.cpp b/gerbview/excellon_read_drill_file.cpp index 2844e58acc..5d7c190d55 100644 --- a/gerbview/excellon_read_drill_file.cpp +++ b/gerbview/excellon_read_drill_file.cpp @@ -772,7 +772,7 @@ bool EXCELLON_IMAGE::Select_Tool( char*& text ) // in tool selection command, if the tool is not defined in list, // and the definition is embedded, it will be entered in list char * startline = text; // the tool id starts here. - int tool_id = TCodeNumber( text ); + int tool_id = CodeNumber( text ); // T0 is legal, but is not a selection tool. it is a special command if( tool_id >= 0 ) diff --git a/gerbview/gerber_file_image.h b/gerbview/gerber_file_image.h index 348448a894..b8f9f8268d 100644 --- a/gerbview/gerber_file_image.h +++ b/gerbview/gerber_file_image.h @@ -222,9 +222,12 @@ public: */ wxPoint ReadIJCoord( char*& Text ); - // functions to read G commands or D commands: - int GCodeNumber( char*& Text ); - int DCodeNumber( char*& Text ); + /** + * Reads the next number and returns the value + * @param aText Pointer to the input string vector + * @return + */ + int CodeNumber( char*& aText ); /** * Return a pointer to the D_CODE within this GERBER for the given \a aDCODE. diff --git a/gerbview/readgerb.cpp b/gerbview/readgerb.cpp index 94b157e8a8..0763227e2c 100644 --- a/gerbview/readgerb.cpp +++ b/gerbview/readgerb.cpp @@ -168,13 +168,13 @@ bool GERBER_FILE_IMAGE::LoadGerberFile( const wxString& aFullFileName ) break; case 'G': /* Line type Gxx : command */ - G_command = GCodeNumber( text ); + G_command = CodeNumber( text ); Execute_G_Command( text, G_command ); break; case 'D': /* Line type Dxx : Tool selection (xx > 0) or * command if xx = 0..9 */ - D_commande = DCodeNumber( text ); + D_commande = CodeNumber( text ); Execute_DCODE_Command( text, D_commande ); break; diff --git a/gerbview/rs274d.cpp b/gerbview/rs274d.cpp index 11f87d01d7..d4bcf80354 100644 --- a/gerbview/rs274d.cpp +++ b/gerbview/rs274d.cpp @@ -396,47 +396,23 @@ static void fillArcPOLY( GERBER_DRAW_ITEM* aGbrItem, const wxPoint& aStart, cons } -int GERBER_FILE_IMAGE::GCodeNumber( char*& Text ) +int GERBER_FILE_IMAGE::CodeNumber( char*& aText ) { - int ii = 0; - char* text; - char line[1024]; + int retval; + char* endptr; - if( Text == nullptr ) + errno = 0; + + retval = strtol( aText + 1, &endptr, 10 ); + + if( endptr == aText || errno != 0 ) return 0; - Text++; - text = line; + wxCHECK_MSG( retval < std::numeric_limits::max(), 0, _( "Invalid Code Number" ) ); - while( IsNumber( *Text ) ) - { - *(text++) = *(Text++); - } + aText = endptr; - *text = 0; - ii = atoi( line ); - return ii; -} - - -int GERBER_FILE_IMAGE::DCodeNumber( char*& Text ) -{ - int ii = 0; - char* text; - char line[1024]; - - if( Text == nullptr ) - return 0; - - Text++; - text = line; - - while( IsNumber( *Text ) ) - *(text++) = *(Text++); - - *text = 0; - ii = atoi( line ); - return ii; + return static_cast( retval ); } @@ -493,7 +469,7 @@ bool GERBER_FILE_IMAGE::Execute_G_Command( char*& text, int G_command ) case GC_SELECT_TOOL: { - int D_commande = DCodeNumber( text ); + int D_commande = CodeNumber( text ); if( D_commande < FIRST_DCODE ) return false;