CVE-2019-15785: Improper Restriction of Operations within the Bounds of a Memory Buffer (CWE-119)
value: FontForge 20190813 through 20190820 has a buffer overflow in PrefsUI_LoadPrefs in prefs.c.
CWE description: The product performs operations on a memory buffer, but it reads from or writes to a memory location outside the buffer's intended boundary. This may result in read or write operations on unexpected memory locations that could be linked to other variables, data structures, or internal program data.
CVSS v3 base score: 9.8
Published: 2019-08-29T13:15Z
Repository: https://github.com/fontforge/fontforge
Language: C
Files changed: fontforge/views.h, fontforgeexe/fontview.c, fontforgeexe/prefs.c, fontforgeexe/scriptingdlg.c, gdraw/gtextfield.c, inc/gdraw.h, inc/ggadget.h
Commit date: 2019-08-13 19:09:59 +0800
Commit message: Warn users before discarding their unsaved scripts (#3852)

* Warn users before discarding their unsaved scripts

This closes #3846.
Diff: diff --git a/fontforge/views.h b/fontforge/views.h
index 55f8c7f78..2cd47ac05 100644
--- a/fontforge/views.h
+++ b/fontforge/views.h
@@ -474,10 +474,11 @@ typedef struct fontview {
     SplineChar *sc_near_top;
     int sel_index;
     struct lookup_subtable *cur_subtable;
     struct qg_data *qg;
     GPid pid_webfontserver;
+    bool script_unsaved; // Whether or not there's an unsaved script in script dialog
 } FontView;
 
 typedef struct findsel {
     GEvent *e;
     real fudge;		/* One pixel fudge factor */
diff --git a/fontforgeexe/fontview.c b/fontforgeexe/fontview.c
index 4ee7e5c32..3f0725bcb 100644
--- a/fontforgeexe/fontview.c
+++ b/fontforgeexe/fontview.c
@@ -78,10 +78,11 @@ int save_to_dir = 0;			/* use sfdir rather than sfd */
 unichar_t *script_menu_names[SCRIPT_MENU_MAX];
 char *script_filenames[SCRIPT_MENU_MAX];
 extern int onlycopydisplayed, copymetadata, copyttfinstr, add_char_to_name_list;
 int home_char='A';
 int compact_font_on_open=0;
+bool warn_script_unsaved = true;
 int navigation_mask = 0;		/* Initialized in startui.c */
 
 static char *fv_fontnames = MONO_UI_FAMILIES;
 extern void python_call_onClosingFunctions();
 
@@ -532,10 +533,26 @@ static int AskChanged(SplineFont *sf) {
     buts[3] = NULL;
     ret = gwwv_ask( _("Font changed"),(const char **) buts,0,2,_("Font %1$.40s in file %2$.40s has been changed.\nDo you want to save it?"),fontname,filename);
 return( ret );
 }
 
+static int AskScriptChanged() {
+    int ret;
+    char *buts[4];
+
+    buts[0] = _("_Yes");
+    buts[1] = _("Yes, and don't _remind me again");
+    buts[2] = _("_No");
+    buts[3] = NULL;
+    ret = gwwv_ask( _("Unsaved script"),(const char **) buts,0,2,_("You have an unsaved script in the «Execute Script» dialog. Do you intend to discard it?"));
+    if (ret == 1) {
+        warn_script_unsaved = false;
+        SavePrefs(true);
+    }
+return( ret );
+}
+
 int _FVMenuGenerate(FontView *fv,int family) {
     FVFlattenAllBitmapSelections(fv);
 return( SFGenerateFont(fv->b.sf,fv->b.active_layer,family,fv->b.normal==NULL?fv->b.map:fv->b.normal) );
 }
 
@@ -856,10 +873,13 @@ static int _FVMenuClose(FontView *fv) {
     if ( !SFCloseAllInstrs(fv->b.sf) )
 return( false );
 
     if ( fv->b.nextsame!=NULL || fv->b.sf->fv!=&fv->b ) {
 	/* There's another view, can close this one with no problems */
+    } else if ( warn_script_unsaved && fv->script_unsaved && 
+                AskScriptChanged()==2 ) {
+        return false;
     } else if ( SFAnyChanged(sf) ) {
 	i = AskChanged(fv->b.sf);
 	if ( i==2 )	/* Cancel */
 return( false );
 	if ( i==0 && !_FVMenuSave(fv))		/* Save */
diff --git a/fontforgeexe/prefs.c b/fontforgeexe/prefs.c
index e5d52d235..4d6c27876 100644
--- a/fontforgeexe/prefs.c
+++ b/fontforgeexe/prefs.c
@@ -192,10 +192,11 @@ extern int prefs_cv_show_control_points_always_initially; /* from charview.c */
 extern int prefs_create_dragging_comparison_outline;      /* from charview.c */
 extern int prefs_cv_outline_thickness; /* from charview.c */
 
 extern float OpenTypeLoadHintEqualityTolerance;  /* autohint.c */
 extern float GenerateHintWidthEqualityTolerance; /* splinesave.c */
+extern bool warn_script_unsaved; /* fontview.c */
 extern NameList *force_names_when_opening;
 extern NameList *force_names_when_saving;
 extern NameList *namelist_for_new_fonts;
 
 extern int default_font_filter_index;
@@ -309,10 +310,11 @@ static struct prefs_list {
 #endif
 	{ N_("ExportClipboard"), pr_bool, &export_clipboard, NULL, NULL, '\0', NULL, 0, N_( "If you are running an X11 clipboard manager you might want\nto turn this off. FF can put things into its internal clipboard\nwhich it cannot export to X11 (things like copying more than\none glyph in the fontview). If you have a clipboard manager\nrunning it will force these to be exported with consequent\nloss of data.") },
 	{ N_("AutoSaveFrequency"), pr_int, &AutoSaveFrequency, NULL, NULL, '\0', NULL, 0, N_( "The number of seconds between autosaves. If you set this to 0 there will be no autosaves.") },
 	{ N_("RevisionsToRetain"), pr_int, &prefRevisionsToRetain, NULL, NULL, '\0', NULL, 0, N_( "When Saving, keep this number of previous versions of the file. file.sfd-01 will be the last saved file, file.sfd-02 will be the file saved before that, and so on. If you set this to 0 then no revisions will be retained.") },
 	{ N_("UndoRedoLimitToSave"), pr_int, &UndoRedoLimitToSave, NULL, NULL, '\0', NULL, 0, N_( "The number of undo and redo operations which will be saved in sfd files.\nIf you set this to 0 undo/redo information is not saved to sfd files.\nIf set to -1 then all available undo/redo information is saved without limit.") },
+	{ N_("WarnScriptUnsaved"), pr_bool, &warn_script_unsaved, NULL, NULL, '\0', NULL, 0, N_( "Whether or not to warn you if you have an unsaved script in the «Execute Script» dialog.") },
 	PREFS_LIST_EMPTY
 },
   new_list[] = {
 	{ N_("NewCharset"), pr_encoding, &default_encoding, NULL, NULL, 'N', NULL, 0, N_("Default encoding for\nnew fonts") },
 	{ N_("NewEmSize"), pr_int, &new_em_size, NULL, NULL, 'S', NULL, 0, N_("The default size of the Em-Square in a newly created font.") },
diff --git a/fontforgeexe/scriptingdlg.c b/fontforgeexe/scriptingdlg.c
index 72ef23ce2..1aa3f4b7d 100644
--- a/fontforgeexe/scriptingdlg.c
+++ b/fontforgeexe/scriptingdlg.c
@@ -187,10 +187,14 @@ static int sd_e_h(GWindow gw, GEvent *event) {
     if ( sd==NULL )
 return( true );
     
     if ( event->type==et_close ) {
 	SD_DoCancel( sd );
+    } else if ( event->type==et_controlevent && event->u.control.subtype==et_textchanged ) {
+    sd->fv->script_unsaved = !GTextFieldIsEmpty(GWidgetGetControl(sd->gw,CID_Script));
+    } else if ( event->type==et_controlevent && event->u.control.subtype==et_save ) {
+    sd->fv->script_unsaved = false;
     } else if ( event->type==et_char ) {
 	if ( event->u.chr.keysym == GK_F1 || event->u.chr.keysym == GK_Help ) {
 	    help("scripting.html");
 return( true );
 	}
diff --git a/gdraw/gtextfield.c b/gdraw/gtextfield.c
index 002ea4c3e..69a4a9e77 100644
--- a/gdraw/gtextfield.c
+++ b/gdraw/gtextfield.c
@@ -213,10 +213,23 @@ static void GTextFieldChanged(GTextField *gt,int src) {
 	(gt->g.handle_controlevent)(&gt->g,&e);
     else
 	GDrawPostEvent(&e);
 }
 
+static void GTextFieldSaved(GTextField *gt) {
+    GEvent e;
+
+    e.type = et_controlevent;
+    e.w = gt->g.base;
+    e.u.control.subtype = et_save;
+    e.u.control.g = &gt->g;
+    if ( gt->g.handle_controlevent != NULL )
+	(gt->g.handle_controlevent)(&gt->g,&e);
+    else
+	GDrawPostEvent(&e);
+}
+
 static void GTextFieldFocusChanged(GTextField *gt,int gained) {
     GEvent e;
 
     if ( (gt->g.box->flags & box_active_border_inner) &&
 	    ( gt->g.state==gs_enabled || gt->g.state==gs_active )) {
@@ -876,10 +889,15 @@ return( space );
 
 static unichar_t txt[] = { '*','.','{','t','x','t',',','p','y','}',  '\0' };
 static unichar_t errort[] = { 'C','o','u','l','d',' ','n','o','t',' ','o','p','e','n',  '\0' };
 static unichar_t error[] = { 'C','o','u','l','d',' ','n','o','t',' ','o','p','e','n',' ','%','.','1','0','0','h','s',  '\0' };
 
+bool GTextFieldIsEmpty(GGadget *g) {
+    GTextField *gt = (GTextField *) g;
+    return gt->text == NULL || *gt->text == '\0';
+}
+
 static void GTextFieldImport(GTextField *gt) {
     unichar_t *ret;
     char *cret;
     unichar_t *str;
 
@@ -968,10 +986,11 @@ return;
 	    putc(*pt>>8,file);
 	    putc(*pt&0xff,file);
 	}
     }
     fclose(file);
+    GTextFieldSaved(gt);
 }
 
 #define MID_Cut		1
 #define MID_Copy	2
 #define MID_Paste	3
diff --git a/inc/gdraw.h b/inc/gdraw.h
index dea28710d..a111207e0 100644
--- a/inc/gdraw.h
+++ b/inc/gdraw.h
@@ -118,11 +118,11 @@ enum visibility_state { vs_unobscured, vs_partially, vs_obscured };
 
 enum et_subtype { et_buttonpress, et_buttonactivate, et_radiochanged,
 		  et_listselected, et_listdoubleclick,
 		  et_scrollbarchange,
 		  et_textchanged, et_textfocuschanged,
-		  et_lastsubtype };
+		  et_save, et_lastsubtype };
 
 enum sb { et_sb_top, et_sb_uppage, et_sb_up, et_sb_left=et_sb_up,
 	  et_sb_down, et_sb_right=et_sb_down, et_sb_downpage,
 	  et_sb_bottom,
 	  et_sb_thumb, et_sb_thumbrelease };
diff --git a/inc/ggadget.h b/inc/ggadget.h
index ae838aee1..fb5e076da 100644
--- a/inc/ggadget.h
+++ b/inc/ggadget.h
@@ -404,10 +404,11 @@ void GGadgetSetHandler(GGadget *g, GGadgetHandler handler);
 GGadgetHandler GGadgetGetHandler(GGadget *g);
 
 void GTextFieldSelect(GGadget *g,int sel_start, int sel_end);
 void GTextFieldShow(GGadget *g,int pos);
 void GTextFieldReplace(GGadget *g,const unichar_t *txt);
+bool GTextFieldIsEmpty(GGadget *g);
 void GCompletionFieldSetCompletion(GGadget *g,GTextCompletionHandler completion);
 void GCompletionFieldSetCompletionMode(GGadget *g,int enabled);
 void GGadgetClearList(GGadget *g);
 void GGadgetSetList(GGadget *g, GTextInfo **ti, int32 copyit);
 GTextInfo **GGadgetGetList(GGadget *g,int32 *len);	/* Do not free!!! */

Vulnerable code: 		  et_lastsubtype };
Fixed code:     bool script_unsaved; // Whether or not there's an unsaved script in script dialog
bool warn_script_unsaved = true;
static int AskScriptChanged() {
    int ret;
    char *buts[4];

    buts[0] = _("_Yes");
    buts[1] = _("Yes, and don't _remind me again");
    buts[2] = _("_No");
    buts[3] = NULL;
    ret = gwwv_ask( _("Unsaved script"),(const char **) buts,0,2,_("You have an unsaved script in the «Execute Script» dialog. Do you intend to discard it?"));
    if (ret == 1) {
        warn_script_unsaved = false;
        SavePrefs(true);
    }
return( ret );
}

    } else if ( warn_script_unsaved && fv->script_unsaved && 
                AskScriptChanged()==2 ) {
        return false;
extern bool warn_script_unsaved; /* fontview.c */
	{ N_("WarnScriptUnsaved"), pr_bool, &warn_script_unsaved, NULL, NULL, '\0', NULL, 0, N_( "Whether or not to warn you if you have an unsaved script in the «Execute Script» dialog.") },
    } else if ( event->type==et_controlevent && event->u.control.subtype==et_textchanged ) {
    sd->fv->script_unsaved = !GTextFieldIsEmpty(GWidgetGetControl(sd->gw,CID_Script));
    } else if ( event->type==et_controlevent && event->u.control.subtype==et_save ) {
    sd->fv->script_unsaved = false;
static void GTextFieldSaved(GTextField *gt) {
    GEvent e;

    e.type = et_controlevent;
    e.w = gt->g.base;
    e.u.control.subtype = et_save;
    e.u.control.g = &gt->g;
    if ( gt->g.handle_controlevent != NULL )
	(gt->g.handle_controlevent)(&gt->g,&e);
    else
	GDrawPostEvent(&e);
}

bool GTextFieldIsEmpty(GGadget *g) {
    GTextField *gt = (GTextField *) g;
    return gt->text == NULL || *gt->text == '\0';
}

    GTextFieldSaved(gt);
		  et_save, et_lastsubtype };
bool GTextFieldIsEmpty(GGadget *g);