trusteddomainproject / OpenDKIM

Other
97 stars 52 forks source link

patch to dkim.c:6467 #108

Open hdatma opened 3 years ago

hdatma commented 3 years ago

I am not sure the compiler is making sense of this.

  /* field bodies are printable ASCII, SP, HT, CR, LF */
  if (!(hdr[c] != 9 ||  /* HT */
        hdr[c] != 10 || /* LF */
        hdr[c] != 13 || /* CR */
        (hdr[c] >= 32 && hdr[c] <= 126) /* SP, print */ ))
   return DKIM_STAT_SYNTAX;

This is what clang 10.0.1 says about it.

dkim.c:6467:22: warning: overlapping comparisons always evaluate to true [-Wtautological-overlap-compare]
                    if (!(hdr[c] != 9 ||  /* HT */
                          ~~~~~~~~~~~~^~~~~~~~~~~~
dkim.c:6470:11: warning: code will never be executed [-Wunreachable-code]
                          (hdr[c] >= 32 && hdr[c] <= 126) /* SP, print */ ))
                           ^~~
dkim.c:6467:20: note: silence by adding parentheses to mark code as explicitly dead
                    if (!(hdr[c] != 9 ||  /* HT */
                                    ^
                                    /* DISABLES CODE */ ( )

It gets better... This is what happens when cleaning it up a bit.

  /* field bodies are printable ASCII, SP, HT, CR, LF */
  if (!(hdr[c] != 9 || hdr[c] != 10 || hdr[c] != 13 || (hdr[c] >= 32 && hdr[c] <= 126) ))
        return DKIM_STAT_SYNTAX;

Ready? Here we go...

dkim.c:6467:23: warning: overlapping comparisons always evaluate to true [-Wtautological-overlap-compare]
                 if ( !(hdr[c] != 9 || hdr[c] != 10 || hdr[c] != 13 || (hdr[c] >= 32 && hdr[c] <= 126) ) )
                        ~~~~~~~~~~~~^~~~~~~~~~~~~~~
dkim.c:6467:59: warning: code will never be executed [-Wunreachable-code]
                 if ( !(hdr[c] != 9 || hdr[c] != 10 || hdr[c] != 13 || (hdr[c] >= 32 && hdr[c] <= 126) ) )
                                                                        ^~~
dkim.c:6467:21: note: silence by adding parentheses to mark code as explicitly dead
                    if ( !(hdr[c] != 9 || hdr[c] != 10 || hdr[c] != 13 || (hdr[c] >= 32 && hdr[c] <= 126) ) )
                                     ^
                                     /* DISABLES CODE */ ( )

I think this is what you wanted to say:

  /* field bodies are printable ASCII, SP, HT, CR, LF */
  if (!( hdr[c] == 9 || hdr[c] == 10 || hdr[c] == 13 || (hdr[c] >= 32 && hdr[c] <= 126) ))
        return DKIM_STAT_SYNTAX;

Proposed patch:

--- ./libopendkim/dkim.c.orig   Thu Nov 15 01:47:38 2018
+++ ./libopendkim/dkim.c        Thu Dec 17 20:21:48 2020
@@ -6464,11 +6464,9 @@
                else
                {
                  /* field bodies are printable ASCII, SP, HT, CR, LF */
-                 if (!(hdr[c] != 9 ||  /* HT */
-                       hdr[c] != 10 || /* LF */
-                       hdr[c] != 13 || /* CR */
-                       (hdr[c] >= 32 && hdr[c] <= 126) /* SP, print */ ))
+                 if (!( hdr[c] == 9 || hdr[c] == 10 || hdr[c] == 13 || (hdr[c] >= 32 && hdr[c] <= 126) )) 
                          return DKIM_STAT_SYNTAX;
                }
        }

@@ -8662,7 +8660,6 @@
 #endif /* _FFR_CONDITIONAL */
 }