bugThe nmh Mail Handling System - Bugs: bug #56192, Unreadable mts.conf silently...


bug #56192: Unreadable mts.conf silently ignored; more general checks required

Submitted by:  Ralph Corderoy <ralph>
Submitted on:  Mon 22 Apr 2019 09:54:26 AM UTC  
Category:  Bug Severity:  3 - Normal
Priority:  5 - Normal Status:  Fixed
Assigned to:  None Open/Closed:  Closed

Add a New Comment (Rich Markup)

You are not logged in

Please log in, so followups can be emailed to you.


Sun 29 Mar 2020 01:03:07 PM UTC, comment #5: 

Looks like I applied the (updated to use nmhetcdir and assign to `path' the first time) patch in commit 1c03fa9d.

David Levine <levine>
Project Member
Wed 24 Apr 2019 12:30:19 AM UTC, comment #4: 

I like it.  A few minor things:  you said it wasn't compiled successfully.  But I couldn't even apply the patch without hacking around a reject of this:
-static char *mtsconf = NMHETCDIR "/mts.conf";
Looks like that should be:
-static char *mtsconf = nmhetcdir(/mts.conf);

The first call to get_optional_env_var() in mts_init() needs its result assigned to `path'.  Or, embed the calls; that might address the following comment, too . . .

The succeeding call to read_mts() isn't protected against a NULL arg, but the following one is.  That corresponds to the second args to get_optional_env_var().  Might that deserve a comment so readers don't have to trace through the called function?

David Levine <levine>
Project Member
Tue 23 Apr 2019 10:55:04 AM UTC, comment #3: 

How about the attached patch, not even compiled successfully here.
It assumes an environment variable set to the empty string is still worthy of attempting to open, i.e. the user will get an error.  I think this is correct as it suggests the user is attempting to set the variable but getting it wrong.  However, it's a change in behaviour.

(file #46812)

Ralph Corderoy <ralph>
Project Member
Tue 23 Apr 2019 12:50:46 AM UTC, comment #2: 

Of course, remove the return after the die().

David Levine <levine>
Project Member
Mon 22 Apr 2019 11:04:56 PM UTC, comment #1: 

Is this sufficient for mts.conf?  The good news is that it's opened in one place, in mts_init().  The bad news is that the function doesn't return a value, so callers can't handle an open failure.

-    if (inited++ || (fp = fopen (get_mtsconf_pathname(), "r")) == NULL)
-       return;
+    if (inited++ || (fp = fopen ((cp = get_mtsconf_pathname()), "r")) == NULL) {
+        die("failed to open %s", cp);
+        return;
+    }

David Levine <levine>
Project Member
Mon 22 Apr 2019 09:54:26 AM UTC, original submission:  

Recent nmh-workers discussion showed a 0600 /etc/nmh/mts.conf,
unreadable by the non-root user, was silently ignored.

In general, nmh needs to tighten up its checking of reading
configuration files.

  • If the file must exist then it is an error if it doesn't.
  • If the file does exist then it is an error if it can't be opened, read

  as required, and closed.

  • If an nmh environment variable is defined to be a path to a file,

  e.g. MHMTSCONF, then the above applies.

Ralph Corderoy <ralph>
Project Member


(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:

Attached Files
file #46812:  nmh-sbr-mts.c.patch added by ralph (3KiB - text/x-patch)


Depends on the following items: None found

Items that depend on this one: None found


Carbon-Copy List
  • -email is unavailable- added by levine (Posted a comment)
  • -email is unavailable- added by ralph (Submitted the item)



    Follow 3 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2020-03-29 levine StatusNone => Fixed
        Open/ClosedOpen => Closed
    2019-04-23 ralph Attached File- => Added nmh-sbr-mts.c.patch, #46812

    Back to the top

    Powered by Savane 3.5