From a8f233bbccfd47a81febb0b6c3ef52bf0a0411a6 Mon Sep 17 00:00:00 2001
From: Robert Sprowson <rsprowson@gitlab.riscosopen.org>
Date: Sat, 26 Jan 2019 22:32:39 +0000
Subject: [PATCH] Various fixes

colourtran.c: Swap round arg names to colourtran_colournumbertoGCOL to match prototype.
txtar.c: Avoid incorrect use of strncat buffer limit, and possible unterminated string after strncpy, by reexpressing using snprintf.
txtedit.c: Avoid incorrect use of strncat buffer limit by reexpressing using snprintf. Avoid potential NULL pointer dereference by moving the filename copy inside the 'if' which confirms it's non-NULL.
txtfind.c: Remove repeated check of repls being valid. Free repls on error. Free pat on error.
xfersend.c: Fix potential out of bounds array access (of leaf[]) if the leafname length exceeds the space in the Wimp message.
Found by cppcheck static analysis.

txtopt.c/h: Sprinkle in some consts.
event.c: Open menu at the Style Guide approved offset of 64.
xfersend.c: Remove redundant check of xfersend__filename being NULL in xfersend__suggest_leaf() as all paths leading to it being called ensure the allocation exists (also, it would have strcpy'd garbage due to malloc not clearing the allocation).

Removed unused header txtover.h.

Version 5.98. Tagged as 'RISC_OSLib-5_98'
---
 VersionASM                | 14 +++++++-------
 VersionNum                | 22 +++++++++++-----------
 rlib/EditIntern/h/txtover | 15 ---------------
 rlib/c/colourtran         |  6 +++---
 rlib/c/event              |  2 +-
 rlib/c/txtar              | 20 ++++----------------
 rlib/c/txtedit            | 11 ++++-------
 rlib/c/txtfind            |  4 +++-
 rlib/c/txtopt             |  6 +++---
 rlib/c/xfersend           |  8 ++++----
 rlib/h/txtopt             |  8 ++++----
 11 files changed, 44 insertions(+), 72 deletions(-)
 delete mode 100644 rlib/EditIntern/h/txtover

diff --git a/VersionASM b/VersionASM
index 52a110c..6e63c38 100644
--- a/VersionASM
+++ b/VersionASM
@@ -11,13 +11,13 @@
                         GBLS    Module_HelpVersion
                         GBLS    Module_ComponentName
                         GBLS    Module_ComponentPath
-Module_MajorVersion     SETS    "5.97"
-Module_Version          SETA    597
+Module_MajorVersion     SETS    "5.98"
+Module_Version          SETA    598
 Module_MinorVersion     SETS    ""
-Module_Date             SETS    "11 Jun 2018"
-Module_ApplicationDate  SETS    "11-Jun-18"
+Module_Date             SETS    "26 Jan 2019"
+Module_ApplicationDate  SETS    "26-Jan-19"
 Module_ComponentName    SETS    "RISC_OSLib"
-Module_ComponentPath    SETS    "castle/RiscOS/Sources/Lib/RISC_OSLib"
-Module_FullVersion      SETS    "5.97"
-Module_HelpVersion      SETS    "5.97 (11 Jun 2018)"
+Module_ComponentPath    SETS    "apache/RiscOS/Sources/Lib/RISC_OSLib"
+Module_FullVersion      SETS    "5.98"
+Module_HelpVersion      SETS    "5.98 (26 Jan 2019)"
                         END
diff --git a/VersionNum b/VersionNum
index ada2af7..8f267d3 100644
--- a/VersionNum
+++ b/VersionNum
@@ -1,23 +1,23 @@
-/* (5.97)
+/* (5.98)
  *
  * This file is automatically maintained by srccommit, do not edit manually.
  * Last processed by srccommit version: 1.1.
  *
  */
-#define Module_MajorVersion_CMHG        5.97
+#define Module_MajorVersion_CMHG        5.98
 #define Module_MinorVersion_CMHG        
-#define Module_Date_CMHG                11 Jun 2018
+#define Module_Date_CMHG                26 Jan 2019
 
-#define Module_MajorVersion             "5.97"
-#define Module_Version                  597
+#define Module_MajorVersion             "5.98"
+#define Module_Version                  598
 #define Module_MinorVersion             ""
-#define Module_Date                     "11 Jun 2018"
+#define Module_Date                     "26 Jan 2019"
 
-#define Module_ApplicationDate          "11-Jun-18"
+#define Module_ApplicationDate          "26-Jan-19"
 
 #define Module_ComponentName            "RISC_OSLib"
-#define Module_ComponentPath            "castle/RiscOS/Sources/Lib/RISC_OSLib"
+#define Module_ComponentPath            "apache/RiscOS/Sources/Lib/RISC_OSLib"
 
-#define Module_FullVersion              "5.97"
-#define Module_HelpVersion              "5.97 (11 Jun 2018)"
-#define Module_LibraryVersionInfo       "5:97"
+#define Module_FullVersion              "5.98"
+#define Module_HelpVersion              "5.98 (26 Jan 2019)"
+#define Module_LibraryVersionInfo       "5:98"
diff --git a/rlib/EditIntern/h/txtover b/rlib/EditIntern/h/txtover
deleted file mode 100644
index acc5d78..0000000
--- a/rlib/EditIntern/h/txtover
+++ /dev/null
@@ -1,15 +0,0 @@
-/* Copyright 2009 Castle Technology Ltd
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-typedef enum {txt_REFUSE, txt_CYCLE, txt_EXTEND} txt_overflowaction;
diff --git a/rlib/c/colourtran b/rlib/c/colourtran
index b67629d..217c53f 100644
--- a/rlib/c/colourtran
+++ b/rlib/c/colourtran
@@ -320,17 +320,17 @@ os_error *colourtran_GCOL_tocolournumber (int gcol, int *col)
 #endif
 
 #ifndef UROM
-os_error *colourtran_colournumbertoGCOL (int gcol, int *col)
+os_error *colourtran_colournumbertoGCOL (int col, int *gcol)
 {
   os_regset r;
   os_error *e;
 
-  r.r[0] = gcol;
+  r.r[0] = col;
 
   e = os_swix(ColourTrans_ColourNumberToGCOL, &r);
 
   if (e == 0)
-    *col = r.r[0];
+    *gcol = r.r[0];
 
   return(e);
 }
diff --git a/rlib/c/event b/rlib/c/event
index 9ef6b3e..81d5e88 100644
--- a/rlib/c/event
+++ b/rlib/c/event
@@ -182,7 +182,7 @@ static BOOL event__process(wimp_eventstr *e)
       }
       event__menux = e->data.but.m.x;
       event__menuy = e->data.but.m.y;
-      wimpt_complain(wimp_create_menu((wimp_menustr*) m, e->data.but.m.x - 48, e->data.but.m.y));
+      wimpt_complain(wimp_create_menu((wimp_menustr*) m, e->data.but.m.x - 64, e->data.but.m.y));
       return FALSE;
     }
   }
diff --git a/rlib/c/txtar b/rlib/c/txtar
index 5c15ba8..45d4a28 100644
--- a/rlib/c/txtar
+++ b/rlib/c/txtar
@@ -573,19 +573,11 @@ void txtar_setoptions(txt t, txtar_options *o /*in*/)
     char a[MAXSYSVARSIZE];
     txtar__sysdata *s = (txtar__sysdata*) t->w->syshandle;
 #ifdef SETOPTIONS
-    strcpy(a, "Set ");
-    strncat(a, txtopt_get_name(), MAXSYSVARSIZE-1);
-    strncat(a, "$Options", MAXSYSVARSIZE-1);
-    sprintf(a+strlen(a), " f%i b%i l%i m%i h%i w%i%s",
-      s->o.forecolour,
-      s->o.backcolour,
-      s->o.leading,
-      s->o.margin,
-      s->o.fontheight,
-      s->o.fontwidth,
-      (s->o.wraptowindow ? " r" : ""));
+    snprintf(a, MAXSYSVARSIZE-1, "Set %s$Options f%i b%i l%i m%i h%i w%i%s",
+      txtopt_get_name(),
 #else
     sprintf(a, "Set Edit$Options f%i b%i l%i m%i h%i w%i%s",
+#endif
       s->o.forecolour,
       s->o.backcolour,
       s->o.leading,
@@ -593,7 +585,6 @@ void txtar_setoptions(txt t, txtar_options *o /*in*/)
       s->o.fontheight,
       s->o.fontwidth,
       (s->o.wraptowindow ? " r" : ""));
-#endif
 #ifdef SET_MISC_OPTIONS
     if (s->o.overwrite) strcat(a, " O");
     if (!s->o.wordtab) strcat(a, " T");
@@ -2342,12 +2333,9 @@ static void txtar__defaultoptions(txtar_options *opt) {
     char buf[MAXSYSVARSIZE];
     int i = 0;
 #ifdef SETOPTIONS
-    char *optname;
     char sysvarname[30];
 
-    optname = txtopt_get_name();
-    strncpy(sysvarname, optname, 30);
-    strncat(sysvarname, "$Options", 30);
+    snprintf(sysvarname, sizeof(sysvarname), "%s$Options", txtopt_get_name());
     os_read_var_val(sysvarname, buf, MAXSYSVARSIZE-1);
 #else
     os_read_var_val("Edit$Options", buf, MAXSYSVARSIZE-1);
diff --git a/rlib/c/txtedit b/rlib/c/txtedit
index 11195d7..ebefd5a 100644
--- a/rlib/c/txtedit
+++ b/rlib/c/txtedit
@@ -3748,12 +3748,9 @@ s->ty = ty;
     int i = 0;
     int undosize = 5000;
 #ifdef SETOPTIONS
-    char *optname;
     char sysvarname[30];
 
-    optname = txtopt_get_name();
-    strncpy(sysvarname, optname, 30);
-    strncat(sysvarname, "$Options", 30);
+    snprintf(sysvarname, sizeof(sysvarname), "%s$Options", txtopt_get_name());
     os_read_var_val(sysvarname, buf, MAXSYSVARSIZE-1);
 #else
     os_read_var_val("Edit$Options", buf, MAXSYSVARSIZE-1);
@@ -3841,8 +3838,9 @@ if (filename != 0 && filename[0] != 0) {
   if (!result) {
     txtedit_dispose(s);
     return NULL;
-  };
-};
+  }
+  strcpy(s->filename, filename);
+}
 
 txt_eventhandler(s->t, txtedit_eventhandler, s);
 
@@ -3853,7 +3851,6 @@ harmless at least. */
 
 txt_setcharoptions(s->t, txt_UPDATED + txt_DISPLAY, txt_DISPLAY);
 txtundo_purge_undo(s->t);
-strcpy(s->filename, filename);
 txtedit_settexttitle(s);
 
 /* IDJ up-call to tell others that a new state has been created 15-Feb-90 */
diff --git a/rlib/c/txtfind b/rlib/c/txtfind
index d618ff1..68fa58c 100644
--- a/rlib/c/txtfind
+++ b/rlib/c/txtfind
@@ -299,6 +299,7 @@ while (rtemplate[i] != '\0')
                        if (ch == 0)
                        {
                            werr(FALSE, msgs_lookup(MSGS_txtfind2));
+                           if (repls) free(repls);
                            return;
                        }
                        if (islower(ch)) ch = toupper(ch);
@@ -446,7 +447,7 @@ repls[j] = '\0';
 txt_setdot(t, start);
 txt_replacechars(t, *end - start, repls, j);
 *end = start + j;
-if (repls) free(repls);
+free(repls);
 }
 
 
@@ -786,6 +787,7 @@ extern Pattern *txtfind_build_pattern(char *pattern, BOOL magic, BOOL nocase
    if (pattern_nfa == NULL)
    {
        werr(FALSE, msgs_lookup(MSGS_txtfind4));
+       free(pat);
        return NULL;
    }
    else
diff --git a/rlib/c/txtopt b/rlib/c/txtopt
index 1ad7a60..5bbfec9 100644
--- a/rlib/c/txtopt
+++ b/rlib/c/txtopt
@@ -37,15 +37,15 @@
 static char *txtopt__option_name = "Edit";
 
 #ifndef UROM
-void txtopt_set_name(char *name)
+void txtopt_set_name(const char *name)
 {
-    if ((txtopt__option_name = malloc(strlen(name)+1)) == 0)
+    if ((txtopt__option_name = malloc(strlen(name)+1)) == NULL)
         werr(TRUE, msgs_lookup(MSGS_txtopt1));
     strcpy(txtopt__option_name, name);
 }
 #endif
 
-char *txtopt_get_name(void)
+const char *txtopt_get_name(void)
 {
     return txtopt__option_name;
 }
diff --git a/rlib/c/xfersend b/rlib/c/xfersend
index 452426c..a61947f 100644
--- a/rlib/c/xfersend
+++ b/rlib/c/xfersend
@@ -111,8 +111,8 @@ static int xfersend__suggest_leaf (wimp_msgdatasave *datasave)
   int i, tail, namelen;
   char name[256];
 
-  if (xfersend__filename == NULL) xfersend__filename = malloc (256);
-  strncpy (name, xfersend__filename, 256);
+  strncpy (name, xfersend__filename, sizeof (name));
+  name[sizeof (name) - 1] = '\0';
   namelen = tail = strlen (name); /* point at the zero */
   while (tail > 0 && name[tail-1] != '.' && name[tail-1] != ':')
     tail--;
@@ -120,10 +120,10 @@ static int xfersend__suggest_leaf (wimp_msgdatasave *datasave)
   for (i = 0; tail <= namelen && i < (sizeof (datasave->leaf) - 1); i++)
     datasave->leaf[i] = name[tail++];
 
-  datasave->leaf[i+1] = '\0';     /* force termination */
+  datasave->leaf[i] = '\0';       /* force termination */
   tracef1 ("suggest leaf '%s'.\n", (int) datasave->leaf);
 
-  return strlen(datasave->leaf) /* name */ + 1 /* terminator */;
+  return i /* name */ + 1 /* terminator */;
 }
 
 static BOOL xfersend__unknowns (wimp_eventstr *e, void *handle)
diff --git a/rlib/h/txtopt b/rlib/h/txtopt
index 0c7d467..9c69121 100644
--- a/rlib/h/txtopt
+++ b/rlib/h/txtopt
@@ -34,8 +34,8 @@
  * Description:   Set the name used as a system variable for setting text
  *                editing options
  *
- * Parameters:    char *name -- the name to be prepended to $Options to form
- *                              the system variable name.
+ * Parameters:    const char *name -- the name to be prepended to $Options to
+ *                                    form the system variable name.
  * Returns:       void.
  * Other Info:    If this function is not called before using any of the
  *                txt and txtedit functions, the system variable name
@@ -45,7 +45,7 @@
  *
  */
 
-void txtopt_set_name(char *name);
+void txtopt_set_name(const char *name);
 
 
 /* ---------------------------- txtopt_get_name ----------------------------
@@ -62,6 +62,6 @@ void txtopt_set_name(char *name);
  *
  */
 
-char *txtopt_get_name(void);
+const char *txtopt_get_name(void);
 
 #endif
-- 
GitLab