On Thu, Mar 02, 2023 at 09:09:31AM -0700, Todd C. Miller wrote:
> On Thu, 02 Mar 2023 09:07:38 -0700, "Theo de Raadt" wrote:
> 
> > +       if (auth.length > total_length)
> >
> > Isn't auth.length a network byte order value?
> 
> Ah yes, good catch; it needs an ntohs().
> 
>  - todd

Hi,

I just looked up RADIUS in RFC 2865 and on page 15 it reads:

--------->
   Length

      The Length field is two octets.  It indicates the length of the
      packet including the Code, Identifier, Length, Authenticator and
      Attribute fields.  Octets outside the range of the Length field
      MUST be treated as padding and ignored on reception.  If the
      packet is shorter than the Length field indicates, it MUST be
      silently discarded.  The minimum length is 20 and maximum length
      is 4096.

<---------

Notice the silent discard, so perhaps we should try again?  I adjusted
the patch for this, but I can't test.

Best Regards,
-peter


Index: raddauth.c
===================================================================
RCS file: /cvs/src/libexec/login_radius/raddauth.c,v
retrieving revision 1.30
diff -u -p -u -r1.30 raddauth.c
--- raddauth.c  28 Jun 2019 13:32:53 -0000      1.30
+++ raddauth.c  2 Mar 2023 16:24:02 -0000
@@ -114,6 +114,7 @@
 char *radius_dir = RADIUS_DIR;
 char auth_secret[MAXSECRETLEN+1];
 volatile sig_atomic_t timedout;
+int silent_discard;
 int alt_retries;
 int retries;
 int sockfd;
@@ -325,7 +326,7 @@ retry:
                        break;
 
                default:
-                       if (timedout)
+                       if (timedout || silent_discard)
                                goto retry;
                        snprintf(_pwstate, sizeof(_pwstate),
                            "invalid response type %d\n", i);
@@ -451,17 +452,24 @@ rad_recv(char *state, char *challenge, u
        struct sockaddr_in sin;
        u_char recv_vector[AUTH_VECTOR_LEN], test_vector[AUTH_VECTOR_LEN];
        MD5_CTX context;
+       ssize_t total_length;
 
        salen = sizeof(sin);
+       silent_discard = 0;
 
        alarm(timeout);
-       if ((recvfrom(sockfd, &auth, sizeof(auth), 0,
-           (struct sockaddr *)&sin, &salen)) < AUTH_HDR_LEN) {
+       total_length = recvfrom(sockfd, &auth, sizeof(auth), 0,
+           (struct sockaddr *)&sin, &salen);
+       alarm(0);
+       if (total_length < AUTH_HDR_LEN) {
                if (timedout)
                        return(-1);
                errx(1, "bogus auth packet from server");
        }
-       alarm(0);
+       if (ntohs(auth.length) > total_length) {
+               silent_discard = 1;
+               return(-1);
+       }
 
        if (sin.sin_addr.s_addr != auth_server)
                errx(1, "bogus authentication server");

Reply via email to