Skip to content

Commit

Permalink
netserver: use mkstemp to create/open debug file
Browse files Browse the repository at this point in the history 
This is another attempt to address the primary issue:
    https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=413658

similar to the original patch proposed:
    https://bugs.debian.org/cgi-bin/bugreport.cgi?att=1;bug=413658;filename=CVE-2007-1444.patch;msg=35
  • Loading branch information
@grundlerchromium
grundlerchromium committed Feb 8, 2018
1 parent d566775 commit 5380b1f
Showing 1 changed file with 14 additions and 11 deletions.
25 changes: 14 additions & 11 deletions src/netserver.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -254,20 +254,23 @@ open_debug_file()
FILE *rd_null_fp;

if (where != NULL) fflush(where);

snprintf(FileName,
sizeof(FileName),
if (suppress_debug) {
FileName = NETPERF_NULL;
where = fopen(Filename, "w");
} else {
snprintf(FileName, sizeof(FileName),
#if defined(WIN32)
"%s\\%s_%d",
getenv("TEMP"),
"%s\\%s_XXXXXX", getenv("TEMP"),
#else
"%s_%d",
"%s_XXXXXX",
#endif
DEBUG_LOG_FILE,
getpid());
if ((where = fopen((suppress_debug) ? NETPERF_NULL : FileName,
"w")) == NULL) {
perror("netserver: debug file");
DEBUG_LOG_FILE);
where = mkstemp(FileName);
}

if (where == NULL) {
fprintf(stderr, "netserver: %s debug file : %s",
FileName, strerror(errno));
exit(1);
}

Expand Down

9 comments on commit 5380b1f

@zman0900
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to be some problems with this:

netserver.c: In function ‘open_debug_file’:
netserver.c:259:14: error: assignment to expression with array type
     FileName = NETPERF_NULL;
              ^
netserver.c:260:19: error: ‘Filename’ undeclared (first use in this function); did you mean ‘FileName’?
     where = fopen(Filename, "w");
                   ^~~~~~~~
                   FileName
netserver.c:260:19: note: each undeclared identifier is reported only once for each function it appears in
netserver.c:264:11: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
     where = mkstemp(FileName);
           ^

@grundlerchromium
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

zman - thanks for checking - obviously I didn't see that on my corp linux machine (debian based). What do I have to do to reproduce this?
Can you post a patch to fix this? (I'd be happy to review and/or test this)

@grundlerchromium
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW, building this results in loads of warnings. Right now I'm only concerned about the error.

@grundlerchromium
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah - pulling the ToT and I see it too. weird. I didn't see this before. I'll post a fix shortly.

@grundlerchromium
Copy link
Contributor Author

@grundlerchromium grundlerchromium commented on 5380b1f Mar 9, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

  -    FileName = NETPERF_NULL;
  -    where = fopen(Filename, "w"); 
  +    strncpy(FileName, NETPERF_NULL, sizeof(FileName));
  +    where = fopen(FileName, "w");

@zman0900
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not great with C, so I'm not sure how much help I can be here. I was trying to compile master for my own use and ran into these 2 errors, which appear to come from this commit. Compile was done on Fedora 27 with gcc 7.3.1. Similar results with clang 5.0.1.

Second error seems to just be a simple wrong case: FileName vs Filename.

I'm not sure about the first error, but searching on "error: assignment to expression with array type" suggests assigning a value to an existing array like that isn't allowed. Probably need to be doing something with pointers, or use strcpy?

@grundlerchromium
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct on both counts. I've posted a pull request here:
grundlerchromium#1

but I'm doing something wrong since it's showing up against my own fork instead HPs original. sigh
I'll sort that out and post a proper pull request.

@grundlerchromium
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#11

@grundlerchromium
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And my apologies - this was simple failure on my part.

Please sign in to comment.