SCP: Fix Coverity identified problems

- Use memmove instead of memcpy for potentially overlapping moves.
- Fix memory leaks in error paths.
- Fix potential NULL pointer references
- Assure that string copies stay within bounds of the destination string and
  are NUL terminated
- Fix NULL pointer dereferencing in help routines
This commit is contained in:
Mark Pizzolato 2017-03-11 02:33:22 -08:00
parent 8df467835a
commit 9731644495

133
scp.c
View file

@ -2928,6 +2928,7 @@ if (flag >= 0) { /* Only bump nesting fro
sim_on_check[sim_do_depth] = 0;
sim_brk_clract (); /* defang breakpoint actions */
--sim_do_depth; /* unwind nesting */
fclose (fpin);
return SCPE_MEM;
}
strcpy(sim_on_actions[sim_do_depth][i], sim_on_actions[sim_do_depth-1][i]);
@ -3515,7 +3516,8 @@ else {
else { /* not reg, check for memory */
if (sim_dfdev && sim_vm_parse_addr) /* get addr */
addr = sim_vm_parse_addr (sim_dfdev, gbuf, &gptr);
else addr = (t_addr) strtotv (gbuf, &gptr, sim_dfdev->dradix);
else
addr = (t_addr) strtotv (gbuf, &gptr, sim_dfdev ? sim_dfdev->dradix : sim_dflt_dev->dradix);
if (gbuf == gptr) /* error? */
return SCPE_NXREG;
}
@ -3759,7 +3761,7 @@ while (1) {
}
}
sim_do_echo = saved_do_echo; /* restore echo mode */
fseek(sim_gotofile, fpos, SEEK_SET); /* resture start position */
fseek(sim_gotofile, fpos, SEEK_SET); /* restore start position */
sim_goto_line[sim_do_depth] = saved_goto_line; /* restore start line number */
return SCPE_ARG;
}
@ -5088,21 +5090,21 @@ if ((*cptr != '/') || (0 == memcmp (cptr, "./", 2)) || (0 == memcmp (cptr, "../"
else
strcpy (WholeName, cptr);
while ((c = strstr (WholeName, "/./")))
strcpy (c + 1, c + 3);
memmove (c + 1, c + 3, 1 + strlen (c + 3));
while ((c = strstr (WholeName, "//")))
strcpy (c + 1, c + 2);
memmove (c + 1, c + 2, 1 + strlen (c + 2));
while ((c = strstr (WholeName, "/../"))) {
char *c1;
c1 = c - 1;
while ((c1 >= WholeName) && (*c1 != '/'))
c1 = c1 - 1;
strcpy (c1, c + 3);
memmove (c1, c + 3, 1 + strlen (c + 3));
while (0 == memcmp (WholeName, "/../", 4))
strcpy (WholeName, WholeName+3);
memmove (WholeName, WholeName+3, 1 + strlen (WholeName+3));
}
c = strrchr (WholeName, '/');
if (c) {
memcpy (DirName, WholeName, 1+c-WholeName);
memmove (DirName, WholeName, 1+c-WholeName);
DirName[1+c-WholeName] = '\0';
}
else {
@ -5344,15 +5346,19 @@ t_stat ssh_break (FILE *st, const char *cptr, int32 flg)
char gbuf[CBUFSIZE], *aptr, abuf[4*CBUFSIZE];
CONST char *tptr, *t1ptr;
DEVICE *dptr = sim_dflt_dev;
UNIT *uptr = dptr->units;
UNIT *uptr;
t_stat r;
t_addr lo, hi, max = uptr->capac - 1;
t_addr lo, hi, max;
int32 cnt;
if (sim_brk_types == 0)
return sim_messagef (SCPE_NOFNC, "No breakpoint support in this simulator\n");
if ((dptr == NULL) || (uptr == NULL))
if (dptr == NULL)
return SCPE_IERR;
uptr = dptr->units;
if (uptr == NULL)
return SCPE_IERR;
max = uptr->capac - 1;
abuf[sizeof(abuf)-1] = '\0';
strncpy (abuf, cptr, sizeof(abuf)-1);
cptr = abuf;
@ -6218,13 +6224,16 @@ for ( ;; ) { /* device loop */
break;
if ((dptr = find_dev (buf)) == NULL) { /* locate device */
sim_printf ("Invalid device name: %s\n", buf);
return SCPE_INCOMP;
r = SCPE_INCOMP;
goto Cleanup_Return;
}
READ_S (buf); /* [V3.0+] logical name */
deassign_device (dptr); /* delete old name */
if ((buf[0] != 0) &&
((r = assign_device (dptr, buf)) != SCPE_OK))
return r;
((r = assign_device (dptr, buf)) != SCPE_OK)) {
r = SCPE_INCOMP;
goto Cleanup_Return;
}
READ_I (flg); /* [V2.10+] ctlr flags */
if (!v32)
flg = ((flg & DEV_UFMASK_31) << (DEV_V_UF - DEV_V_UF_31)) |
@ -6238,7 +6247,8 @@ for ( ;; ) { /* device loop */
break;
if ((uint32) unitno >= dptr->numunits) { /* too big? */
sim_printf ("Invalid unit number: %s%d\n", sim_dname (dptr), unitno);
return SCPE_INCOMP;
r = SCPE_INCOMP;
goto Cleanup_Return;
}
READ_I (time); /* event time */
uptr = (dptr->units) + unitno;
@ -6272,8 +6282,11 @@ for ( ;; ) { /* device loop */
if ((uptr->flags & UNIT_ATT) && /* unit currently attached? */
(!dont_detach_attach)) {
r = scp_detach_unit (dptr, uptr); /* detach it */
if (r != SCPE_OK)
return sim_messagef (r, "Error detaching %s from %s: %s\n", sim_uname (uptr), uptr->filename, sim_error_text (r));
if (r != SCPE_OK) {
sim_printf ("Error detaching %s from %s: %s\n", sim_uname (uptr), uptr->filename, sim_error_text (r));
r = SCPE_INCOMP;
goto Cleanup_Return;
}
}
if ((buf[0] != '\0') && /* unit to be reattached? */
((uptr->flags & UNIT_ATTABLE) || /* and unit is attachable */
@ -6296,7 +6309,8 @@ for ( ;; ) { /* device loop */
if (((uptr->flags & (UNIT_FIX + UNIT_ATTABLE)) != UNIT_FIX) ||
(dptr->deposit == NULL)) {
sim_printf ("Can't restore memory: %s%d\n", sim_dname (dptr), unitno);
return SCPE_INCOMP;
r = SCPE_INCOMP;
goto Cleanup_Return;
}
if (high != old_capac) { /* size change? */
uptr->capac = old_capac; /* temp restore old */
@ -6305,7 +6319,8 @@ for ( ;; ) { /* device loop */
(dptr->msize (uptr, (int32) high, NULL, NULL) != SCPE_OK))) {
sim_printf ("Can't change memory size: %s%d\n",
sim_dname (dptr), unitno);
return SCPE_INCOMP;
r = SCPE_INCOMP;
goto Cleanup_Return;
}
uptr->capac = high; /* new memory size */
sim_printf ("Memory size changed: %s%d = ", sim_dname (dptr), unitno);
@ -6315,19 +6330,23 @@ for ( ;; ) { /* device loop */
sim_printf ("\n");
}
sz = SZ_D (dptr); /* allocate buffer */
if ((mbuf = calloc (SRBSIZ, sz)) == NULL)
return SCPE_MEM;
if ((mbuf = calloc (SRBSIZ, sz)) == NULL) {
r = SCPE_MEM;
goto Cleanup_Return;
}
for (k = 0; k < high; ) { /* loop thru mem */
if (sim_fread (&blkcnt, sizeof (blkcnt), 1, rfile) == 0) {/* block count */
free (mbuf);
return SCPE_IOERR;
r = SCPE_IOERR;
goto Cleanup_Return;
}
if (blkcnt < 0) /* compressed? */
limit = -blkcnt;
else limit = (int32)sim_fread (mbuf, sz, blkcnt, rfile);
if (limit <= 0) { /* invalid or err? */
free (mbuf);
return SCPE_IOERR;
r = SCPE_IOERR;
goto Cleanup_Return;
}
for (j = 0; j < limit; j++, k = k + (dptr->aincr)) {
if (blkcnt < 0) /* compressed? */
@ -6336,7 +6355,7 @@ for ( ;; ) { /* device loop */
r = dptr->deposit (val, k, uptr, SIM_SW_REST);
if (r != SCPE_OK) {
free (mbuf);
return r;
goto Cleanup_Return;
}
} /* end for j */
} /* end for k */
@ -6358,6 +6377,8 @@ for ( ;; ) { /* device loop */
if (depth != rptr->depth) { /* [V2.10+] mismatch? */
sim_printf ("Register depth mismatch: %s %s, file = %d, sim = %d\n",
sim_dname (dptr), buf, depth, rptr->depth);
if (depth > rptr->depth)
depth = rptr->depth;
}
mask = width_mask[rptr->width]; /* get mask */
for (us = 0; us < depth; us++) { /* loop thru values */
@ -6409,6 +6430,7 @@ for (j=0, r = SCPE_OK; j<attcnt; j++) {
}
free (attnames[j]);
}
Cleanup_Return:
free (attnames);
free (attunits);
free (attswitches);
@ -6438,7 +6460,7 @@ t_stat run_cmd (int32 flag, CONST char *cptr)
char gbuf[CBUFSIZE] = "";
CONST char *tptr;
uint32 i, j;
int32 sim_next;
int32 sim_next = 0;
int32 unitno;
t_value pcv, orig_pcv;
t_stat r;
@ -6526,7 +6548,8 @@ else if (flag == RU_NEXT) { /* next */
if ((r != SCPE_OK) || (sim_next <= 0)) /* error? */
return SCPE_ARG;
}
else sim_next = 1;
else
sim_next = 1;
if (sim_vm_is_subroutine_call(&addrs)) {
sim_brk_types |= BRK_TYP_DYN_STEPOVER;
for (i=0; addrs[i]; i++)
@ -9552,13 +9575,16 @@ do {
sim_brk_ins = p;
return bp;
}
else if (loc < bp->addr) /* go down? p is upper */
hi = p - 1;
else lo = p + 1; /* go up? p is lower */
else
if (loc < bp->addr) /* go down? p is upper */
hi = p - 1;
else
lo = p + 1; /* go up? p is lower */
} while (lo <= hi);
if (loc < bp->addr) /* insrt before or */
sim_brk_ins = p;
else sim_brk_ins = p + 1; /* after last sch */
else
sim_brk_ins = p + 1; /* after last sch */
return NULL;
}
@ -9663,7 +9689,8 @@ return SCPE_OK;
t_stat sim_brk_clr (t_addr loc, int32 sw)
{
BRKTAB *bpl, *bp = sim_brk_fnd (loc);
BRKTAB *bpl = NULL;
BRKTAB *bp = sim_brk_fnd (loc);
int32 i;
if (!bp) /* not there? ok */
@ -9675,9 +9702,9 @@ while (bp) {
if (bp->typ == (bp->typ & sw)) {
free (bp->act); /* deallocate action */
if (bp == sim_brk_tab[sim_brk_ins])
bpl = sim_brk_tab[sim_brk_ins] = bp->next;
bpl = sim_brk_tab[sim_brk_ins] = bp->next; /* remove from head of list */
else
bpl->next = bp->next;
bpl->next = bp->next; /* remove from middle of list */
free (bp);
bp = bpl;
}
@ -10205,6 +10232,7 @@ ep->switches = switches; /* set switches */
match_buf = (uint8 *)calloc (strlen (match) + 1, 1);
if ((match_buf == NULL) || (ep->match_pattern == NULL)) {
sim_exp_clr_tab (exp, ep); /* clear it */
free (match_buf);
return SCPE_MEM;
}
if (switches & EXP_TYP_REGEX) {
@ -10228,7 +10256,8 @@ if (ep->act) { /* replace old action? *
free (ep->act); /* deallocate */
ep->act = NULL; /* now no action */
}
if (act) while (sim_isspace(*act)) ++act; /* skip leading spaces in action string */
if (act)
while (sim_isspace(*act)) ++act; /* skip leading spaces in action string */
if ((act != NULL) && (*act != 0)) { /* new action? */
char *newp = (char *) calloc (strlen (act)+1, sizeof (*act)); /* alloc buf */
if (newp == NULL) /* mem err? */
@ -10589,7 +10618,7 @@ int32 cond;
*stat = SCPE_ARG;
cptr = get_glyph (cptr, gbuf, 0);
if (0 == memcmp("SCPE_", gbuf, 5))
strcpy (gbuf, gbuf+5); /* skip leading SCPE_ */
memmove (gbuf, gbuf+5, 1 + strlen (gbuf+5));/* skip leading SCPE_ */
for (cond=0; cond < (SCPE_MAX_ERR-SCPE_BASE); cond++)
if (0 == strcmp(scp_errors[cond].code, gbuf)) {
cond += SCPE_BASE;
@ -11548,24 +11577,27 @@ if (!tmp) {
return;
}
if (topic->title)
if (topic->title) {
fprintf (st, "%s\n", topic->title+1);
skiplines = 0;
if (!strcmp (topic->title+1, "Registers")) {
fprint_reg_help (tmp, dptr) ;
skiplines = 1;
}
else
if (!strcmp (topic->title+1, "Set commands")) {
fprint_set_help (tmp, dptr);
skiplines = 3;
}
else
if (!strcmp (topic->title+1, "Show commands")) {
fprint_show_help (tmp, dptr);
skiplines = 3;
skiplines = 0;
if (dptr) {
if (!strcmp (topic->title+1, "Registers")) {
fprint_reg_help (tmp, dptr) ;
skiplines = 1;
}
else
if (!strcmp (topic->title+1, "Set commands")) {
fprint_set_help (tmp, dptr);
skiplines = 3;
}
else
if (!strcmp (topic->title+1, "Show commands")) {
fprint_show_help (tmp, dptr);
skiplines = 3;
}
}
}
rewind (tmp);
/* Discard leading blank lines/redundant titles */
@ -11931,7 +11963,8 @@ if (fp == NULL) {
* of the executable. Failing that, try the 'help' subdirectory
* of the executable. Failing that, we're out of luck.
*/
strncpy (fbuf, sim_argv[0], sizeof (fbuf));
fbuf[sizeof(fbuf)-1] = '\0';
strncpy (fbuf, sim_argv[0], sizeof (fbuf)-1);
if ((p = (char *)match_ext (fbuf, "EXE")))
*p = '\0';
if ((p = strrchr (fbuf, '\\'))) {