CodeQL File{MayNot,Never}BeClosed.ql file-handle resource leaks

Guided by CodeQL static code analyser.

FileMayNotBeClosed.ql
FileMayNeverBeClosed.ql

The trick with "if(fp != stdout)" is problematic (to analyser) as
technically 'stdout' can be a global pointer that COULD be modified any
time, so it might have changed between the fopen() and fclose() calls so
the close MAY NEVER occurs (which is problem the analyzer can see).

So local state is maintained as a bool which will also clarify to the
compiler see the intention without concern for external stdout
modification.

Some items appear to be out and out leaks when certain commands are use.
This commit is contained in:
Darryl L. Miles 2025-02-13 08:22:28 +00:00 committed by Tim Edwards
parent 7960020f7c
commit e88dcba1c5
10 changed files with 104 additions and 55 deletions

View File

@ -3994,7 +3994,6 @@ CmdDrc(
MagWindow *w,
TxCommand *cmd)
{
FILE * fp;
static int drc_nth = 1;
int option, result, radius;
Rect rootArea, area;
@ -4475,15 +4474,19 @@ CmdDrc(
case PRINTRULES:
if (argc > 3) goto badusage;
if (argc < 3)
fp = stdout;
else if ((fp = fopen (argv[2],"w")) == (FILE *) NULL)
{
DRCPrintRulesTable (stdout);
}
{
FILE *fp = fopen (argv[2], "w");
if (fp == NULL)
{
TxError("Cannot write file %s\n", argv[2]);
return;
}
DRCPrintRulesTable (fp);
if (fp != stdout)
(void) fclose(fp);
fclose(fp);
}
break;
case RULESTATS:

View File

@ -1821,6 +1821,7 @@ CmdWire(
TxError("Bad coordinate pair at %s line %d\n",
cmd->tx_argv[4], i + 1);
freeMagic(plist);
fclose(pfile);
return;
}

View File

@ -119,7 +119,6 @@ ExtractTest(w, cmd)
CellUse *selectedCell;
Rect editArea;
char *addr, *name;
FILE *f;
typedef enum { CLRDEBUG, CLRLENGTH, DRIVER, DUMP, INTERACTIONS,
INTERCOUNT, EXTPARENTS, RECEIVER, SETDEBUG, SHOWDEBUG,
SHOWPARENTS, SHOWTECH, STATS, STEP, TIME } cmdType;
@ -215,37 +214,41 @@ ExtractTest(w, cmd)
DBClearPaintPlane(interPlane);
break;
case INTERCOUNT:
f = stdout;
halo = 1;
if (cmd->tx_argc > 2)
halo = atoi(cmd->tx_argv[2]);
if (cmd->tx_argc > 3)
{
f = fopen(cmd->tx_argv[3], "w");
FILE *f = fopen(cmd->tx_argv[3], "w");
if (f == NULL)
{
perror(cmd->tx_argv[3]);
break;
}
}
ExtInterCount((CellUse *) w->w_surfaceID, halo, f);
if (f != stdout)
(void) fclose(f);
fclose(f);
}
else
{
ExtInterCount((CellUse *) w->w_surfaceID, halo, stdout);
}
break;
case TIME:
f = stdout;
if (cmd->tx_argc > 2)
{
f = fopen(cmd->tx_argv[2], "w");
FILE *f = fopen(cmd->tx_argv[2], "w");
if (f == NULL)
{
perror(cmd->tx_argv[2]);
break;
}
}
ExtTimes((CellUse *) w->w_surfaceID, f);
if (f != stdout)
(void) fclose(f);
fclose(f);
}
else
{
ExtTimes((CellUse *) w->w_surfaceID, stdout);
}
break;
case EXTPARENTS:
if (ToolGetEditBox(&editArea))
@ -1105,6 +1108,7 @@ ExtDumpCaps(filename)
return;
}
ExtDumpCapsToFile(f);
fclose(f);
return;
}

View File

@ -122,7 +122,6 @@ char *path; /* a search path */
char *libPath; /* a library search path */
{
FILE *f;
int max, red, green, blue, newmax, argc, i;
colorEntry *ce;
char fullName[256], inputLine[128], colorName[100];
@ -135,7 +134,7 @@ char *libPath; /* a library search path */
(void) sprintf(fullName, "%.80s.%.80s.%.80s", techStyle,
dispType, monType);
f = PaOpen(fullName, "r", ".cmap", path, libPath, (char **) NULL);
FILE *f = PaOpen(fullName, "r", ".cmap", path, libPath, (char **) NULL);
if (f == NULL)
{
/* Check for original ".cmap1" file (prior to magic v. 7.2.27) */
@ -163,7 +162,7 @@ char *libPath; /* a library search path */
TxError("Syntax error in color map file \"%s.cmap\"\n", fullName);
TxError("Last color read was index %d\n", max);
return FALSE;
goto cleanup;
}
else
{
@ -219,6 +218,11 @@ char *libPath; /* a library search path */
GrSetCMap();
return TRUE;
cleanup:
if(f)
fclose(f);
return FALSE;
}

View File

@ -576,7 +576,7 @@ PlotPNM(fileName, scx, layers, xMask, width)
* plot, in pixels.
*/
{
FILE *fp;
FILE *fp = NULL;
Rect bbox;
int bb_ysize, bb_xsize;
int i, x, y, tile_ydelta;
@ -920,6 +920,7 @@ PlotPNM(fileName, scx, layers, xMask, width)
}
fflush(rtl_args.outfile);
fclose(rtl_args.outfile);
rtl_args.outfile = NULL;
freeMagic(rtl_args.outbytes);
/* Run spooler */
@ -932,7 +933,13 @@ PlotPNM(fileName, scx, layers, xMask, width)
}
else
#endif
{
if(fp)
{
fclose (fp);
fp = NULL;
}
}
done:
PlotPNMdownsample = save_ds;
@ -941,6 +948,15 @@ done:
freeMagic(strip);
freeMagic(lkstep);
lkstep = NULL;
#ifdef VERSATEC
if(rtl_args.outfile) /* theoretical fp leak */
{
fclose(rtl_args.outfile);
rtl_args.outfile = NULL;
}
#endif
if(fp)
fclose(fp);
return;
}

View File

@ -1135,7 +1135,6 @@ PlotPS(fileName, scx, layers, xMask)
{
int xsize, ysize;
float yscale;
FILE *infile;
int i, j;
int twidth, theight;
char *fontptr, *fptr2, *fptr3;
@ -1207,12 +1206,17 @@ PlotPS(fileName, scx, layers, xMask)
/* Insert the prolog here */
infile = PaOpen("magicps", "r", ".pro", ".", SysLibPath, NULL);
{
FILE *infile = PaOpen("magicps", "r", ".pro", ".", SysLibPath, NULL);
if (infile != NULL)
{
while(fgets(line_in, 99, infile) != NULL)
fputs(line_in, file);
fclose(infile);
}
else
fprintf(file, "\npostscript_prolog_is_missing\n\n");
}
/* Insert the font definitions here. */

View File

@ -1353,7 +1353,7 @@ PlotVersatec(scx, layers, xMask, user_scale)
{
TxError("Warning: No color versatec styles are defined"
" in the technology file!\nPlotting aborted.\n");
return;
goto error_close_only;
}
break;
default:
@ -1372,7 +1372,7 @@ PlotVersatec(scx, layers, xMask, user_scale)
TxError("Warning: No monochrome versatec styles are"
" defined in the technology file!\nPlotting"
" aborted.\n");
return;
goto error_close_only;
}
for ( ; curStyle != NULL; curStyle = curStyle->vs_next)
@ -1479,6 +1479,10 @@ PlotVersatec(scx, layers, xMask, user_scale)
TxError("\nVersatec plot aborted.\n");
fclose(file);
unlink(fileName);
return;
error_close_only:
fclose(file);
}
#endif /* VERSATEC */

View File

@ -143,7 +143,6 @@ PlowTest(w, cmd)
Plane *plane;
Edge edge;
Tile *tp;
FILE *f;
if (!ToolGetEditBox(&editArea) || !ToolGetBox(&rootBoxDef, &rootBox))
return;
@ -244,19 +243,21 @@ PlowTest(w, cmd)
plowYankDef = saveDef;
break;
case PC_TECHSHOW:
f = stdout;
if (cmd->tx_argc >= 3)
{
f = fopen(cmd->tx_argv[2], "w");
FILE *f = fopen(cmd->tx_argv[2], "w");
if (f == NULL)
{
perror(cmd->tx_argv[2]);
break;
}
}
plowTechShow(f);
if (f != stdout)
(void) fclose(f);
fclose(f);
}
else
{
plowTechShow(stdout);
}
break;
case PC_WIDTH:
if (cmd->tx_argc < 3)

View File

@ -216,6 +216,7 @@ ResReadSim(simfile, fetproc, capproc, resproc, attrproc, mergeproc, subproc)
if (result != 0)
{
TxError("Error in sim file %s\n", line[0]);
fclose(fp);
return 1;
}
}
@ -922,6 +923,7 @@ ResSimProcessDrivePoints(filename)
node->drivepoint.p_y = atoi(line[RES_EXT_ATTR_Y]);
node->rs_ttype = DBTechNoisyNameType(line[RES_EXT_ATTR_TILE]);
}
fclose(fp);
}
/*
@ -982,6 +984,7 @@ ResSimProcessFixPoints(filename)
thisfix->fp_tile = NULL;
strcpy(thisfix->fp_name, label);
}
fclose(fp);
}

View File

@ -340,7 +340,9 @@ badWarn:
if (!ToolGetEditBox(&editArea))
return;
channame = cmd->tx_argv[2];
f = stdout;
{
bool need_close;
FILE *f;
if (cmd->tx_argc == 4)
{
f = fopen(cmd->tx_argv[3], "w");
@ -349,6 +351,12 @@ badWarn:
perror(cmd->tx_argv[3]);
return;
}
need_close = TRUE;
}
else
{
f = stdout;
need_close = FALSE;
}
if (channame[0] == 'h') GAGenChans(CHAN_HRIVER, &editArea, f);
else if (channame[0] == 'v') GAGenChans(CHAN_VRIVER, &editArea, f);
@ -357,8 +365,9 @@ badWarn:
TxError("Unrecognized channel type: %s\n", channame);
TxError("Legal types are \"h\" or \"v\"\n");
}
if (f != stdout)
if (need_close)
(void) fclose(f);
}
break;
case CHANNEL:
channame = (char *) NULL;