On 2 January 2012 06:58, Henning Brauer <[email protected]> wrote:
> what next? having pfctl whine about an empty config file?
>
> * Stephane A. Sezer <[email protected]> [2012-01-02 09:36]:
>> Hi,
>>
>> I found a strange behavior in pfctl(8) which looks like a bug.
>>
>> When given a directory as input (either with the `-f` flag, or with the
>> `include` directive in a config file), pfctl(8) does not emit any
>> warning and silently accepts the given input.
>>
>> I suppose this is not the intended behavior so here is a patch that
>> fixes the issue.
Even if it is the case, testing against directory doesn't seem like
the right thing, the test should be done against a regular file, what
if you pass a block device ? another check ?
You're only dealing with a very special case here, anyway, I agree
with Henning, it seems too much.
>>
>> Regards,
>>
>> --
>> Stephane A. Sezer
>>
>> Index: sbin/pfctl/parse.y
>> ===================================================================
>> RCS file: /cvs/src/sbin/pfctl/parse.y,v
>> retrieving revision 1.613
>> diff -u -r1.613 parse.y
>> --- sbin/pfctl/parse.y 19 Dec 2011 23:26:16 -0000 1.613
>> +++ sbin/pfctl/parse.y 2 Jan 2012 08:04:12 -0000
>> @@ -5614,37 +5614,52 @@
>> pushfile(const char *name, int secret)
>> {
>> struct file *nfile;
>> + struct stat sb;
>>
>> - if ((nfile = calloc(1, sizeof(struct file))) == NULL ||
>> - (nfile->name = strdup(name)) == NULL) {
>> - if (nfile)
>> - free(nfile);
>> - warn("malloc");
>> - return (NULL);
>> + if ((nfile = calloc(1, sizeof(struct file))) == NULL) {
>> + warn("calloc");
>> + goto err;
>> }
>> - if (TAILQ_FIRST(&files) == NULL && strcmp(nfile->name, "-") == 0) {
>> +
>> + if (TAILQ_FIRST(&files) == NULL && strcmp(name, "-") == 0) {
>> nfile->stream = stdin;
>> - free(nfile->name);
>> if ((nfile->name = strdup("stdin")) == NULL) {
>> warn("strdup");
>> - free(nfile);
>> - return (NULL);
>> + goto err;
>> + }
>> + } else {
>> + if ((nfile->name = strdup(name)) == NULL) {
>> + warn("strdup");
>> + goto err;
>> + }
>> + if ((nfile->stream = fopen(nfile->name, "r")) == NULL) {
>> + warn("%s", nfile->name);
>> + goto err;
>> + }
>> + if (secret &&
>> + check_file_secrecy(fileno(nfile->stream), nfile->name))
>> + goto file_err;
>> + if (fstat(fileno(nfile->stream), &sb) == -1) {
>> + warn("fstat");
>> + goto file_err;
>> + }
>> + if (S_ISDIR(sb.st_mode)) {
>> + errno = EISDIR;
>> + warn("%s", nfile->name);
>> + goto file_err;
>> }
>> - } else if ((nfile->stream = fopen(nfile->name, "r")) == NULL) {
>> - warn("%s", nfile->name);
>> - free(nfile->name);
>> - free(nfile);
>> - return (NULL);
>> - } else if (secret &&
>> - check_file_secrecy(fileno(nfile->stream), nfile->name)) {
>> - fclose(nfile->stream);
>> - free(nfile->name);
>> - free(nfile);
>> - return (NULL);
>> }
>> +
>> nfile->lineno = 1;
>> TAILQ_INSERT_TAIL(&files, nfile, entry);
>> return (nfile);
>> +
>> +file_err:
>> + fclose(nfile->stream);
>> +err:
>> + free(nfile->name);
>> + free(nfile);
>> + return (NULL);
>> }
>>
>> int
>>
>
> --
> Henning Brauer, [email protected], [email protected]
> BS Web Services, http://bsws.de, Full-Service ISP
> Secure Hosting, Mail and DNS Services. Dedicated Servers, Root to Fully
Managed
> Henning Brauer Consulting, http://henningbrauer.com/
