Wietse Venema via Postfix-devel:
> > There is an unnecessary double check of the "addr_list" pointer for null
> 
> (twice). It does no harm, and a good compiler will remove that test.
> Generally, I don't worry about repeated tests like this. They make
> Postfix code a bit easier to maintain, because there are fewer
> implicit dependencies.
> 
> > In posttls-finger there are problems with using "service" char pointer in
> > logging. I initialized the variable
> > to null and ensured that it does not point into already freed space.
> 
> Yeah, that is the result of clumsy memory management and should be fixed. 
> 
> I'll try to make this more obviously correct without adding multiple
> instances of "if (bah != 0) myfree(bah);" throughout the code,
> because it's too easy to forget adding another instance when the
> code is updated.

I simplified the code as with patch below. This 1) fixes the warning
message when the destination in "dest" contains ":service" information,
and 2) limits the visibility of some temporary variables.

        Wietse

20230517

        Bugfix (defect introduced: Postfix 3.8) the posttls-finger
        command could access uninitialized memory when reconnecting.
        Reported by Thomas Korbar. File: posttls-finger/posttls-finger.c.

diff '--exclude=man' '--exclude=html' '--exclude=README_FILES' 
'--exclude=INSTALL' '--exclude=.indent.pro' -r -ur 
/var/tmp/postfix-3.9-20230516/src/posttls-finger/posttls-finger.c 
./src/posttls-finger/posttls-finger.c
--- /var/tmp/postfix-3.9-20230516/src/posttls-finger/posttls-finger.c   
2023-04-13 10:16:06.000000000 -0400
+++ ./src/posttls-finger/posttls-finger.c       2023-05-16 13:46:27.593011475 
-0400
@@ -1590,12 +1590,13 @@
 static void connect_remote(STATE *state, char *dest)
 {
     DNS_RR *addr;
-    char   *buf;
-    char   *domain;
-    char   *service;
 
     /* When reconnecting use IP address of previous session */
     if (state->addr == 0) {
+       char   *buf;
+       char   *domain;
+       char   *service;
+
        buf = parse_destination(dest, state->smtp ? "smtp" : "24",
                                &domain, &service, &state->port);
        if (!state->nexthop)
@@ -1622,8 +1623,8 @@
 
        if (level == TLS_LEV_INVALID
            || (state->stream = connect_addr(state, addr)) == 0) {
-           msg_info("Failed to establish session to %s:%s via %s:%u: %s",
-                    dest, service, HNAME(addr), addr->port,
+           msg_info("Failed to establish session to %s via %s:%u: %s",
+                    dest, HNAME(addr), addr->port,
                     vstring_str(state->why->reason));
            continue;
        }
_______________________________________________
Postfix-devel mailing list -- postfix-devel@postfix.org
To unsubscribe send an email to postfix-devel-le...@postfix.org

Reply via email to