Skip to content

Commit

Permalink
fix: Out-of-bounds memory access of filename.
Browse files Browse the repository at this point in the history
Put all the filename allocation and deallocation into the `gerb_fopen`
and `gerb_fclose` functions so that the caller doesn't need to deal
with this anymore.

Also, free includeFilename.  It was allocated as part of
`gerb_fgetstring()` above but never freed properly.

Unit tests added to valgrind.
  • Loading branch information
eyal0 authored and ooxi committed Jul 13, 2023
1 parent 14f9df4 commit 5517e22
Show file tree
Hide file tree
Showing 8 changed files with 23 additions and 6 deletions.
5 changes: 5 additions & 0 deletions src/gerb_file.c
Expand Up @@ -131,6 +131,9 @@ gerb_fopen(char const * filename)

#endif

dprintf(" Setting filename\n");
fd->filename = g_strdup(filename);

dprintf("<--- Leaving gerb_fopen\n");
return fd;
} /* gerb_fopen */
Expand Down Expand Up @@ -238,6 +241,8 @@ void
gerb_fclose(gerb_file_t *fd)
{
if (fd) {
g_free(fd->filename);

#ifdef HAVE_SYS_MMAN_H
if (munmap(fd->data, fd->datalen) < 0)
GERB_FATAL_ERROR("munmap: %s", strerror(errno));
Expand Down
2 changes: 1 addition & 1 deletion src/gerber.c
Expand Up @@ -1476,7 +1476,7 @@ parse_rs274x(gint levelOfRecursion, gerb_file_t *fd, gerbv_image_t *image,
"include file recursion which is not allowed "
"by the RS-274X spec"));
}

g_free (includeFilename);
}
}
break;
Expand Down
4 changes: 0 additions & 4 deletions src/gerbv.c
Expand Up @@ -507,9 +507,6 @@ gerbv_open_image(gerbv_project_t *gerbvProject, gchar const* filename, int idx,
return -1;
}

/* Store filename info fd for further use */
fd->filename = g_strdup(filename);

dprintf("In open_image, successfully opened file. Now check its type....\n");
/* Here's where we decide what file type we have */
/* Note: if the file has some invalid characters in it but still appears to
Expand Down Expand Up @@ -578,7 +575,6 @@ gerbv_open_image(gerbv_project_t *gerbvProject, gchar const* filename, int idx,
parsed_image = NULL;
}

g_free(fd->filename);
gerb_fclose(fd);
if (parsed_image == NULL) {
return -1;
Expand Down
Binary file added test/golden/parse_aperture_strtok.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
11 changes: 11 additions & 0 deletions test/inputs/parse_aperture_strtok.1.poc
@@ -0,0 +1,11 @@
%FSLAX25Y25*%
%MOIN*%

%ADD20C,BBBB*%

%IFparse_aperture_strtok.2.poc*%

D21*
X400000Y400000D03*

M02*
2 changes: 2 additions & 0 deletions test/inputs/parse_aperture_strtok.2.poc
@@ -0,0 +1,2 @@
M09*

3 changes: 2 additions & 1 deletion test/run_valgrind_tests.sh
Expand Up @@ -2,4 +2,5 @@
./run_tests.sh --valgrind \
example_cslk \
out-of-bounds-drill-tool \
example_am_test
example_am_test \
parse_aperture_strtok
2 changes: 2 additions & 0 deletions test/tests.list
Expand Up @@ -182,6 +182,8 @@ test-circular-interpolation-zero-error | test-circular-interpolation-zero-erro
test-merge-a+b_temporary | test-merge-a.gbx test-merge-b.gbx | ! --export=rs274x --output=inputs/test-merge-a+b_temporary.gbx
test-merge-a+b | test-merge-a+b_temporary.gbx

parse_aperture_strtok | parse_aperture_strtok.1.poc

# ---------------------------------------------
# Excellon drill tests
# ---------------------------------------------
Expand Down

0 comments on commit 5517e22

Please sign in to comment.