Date: Sat, 11 Sep 2004 16:46:20 +0200 (CEST) From: Dan Lukes <dan@obluda.cz> To: FreeBSD-gnats-submit@FreeBSD.org Subject: bin/71592: [PATCH] ppp doesn't check mallocs for return of NULL Message-ID: <200409111446.i8BEkKNT001836@kulesh.obluda.cz> Resent-Message-ID: <200409111450.i8BEoOql097376@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>Number: 71592
>Category: bin
>Synopsis: [PATCH] ppp doesn't check mallocs for return of NULL
>Confidential: no
>Severity: serious
>Priority: medium
>Responsible: freebsd-bugs
>State: open
>Quarter:
>Keywords:
>Date-Required:
>Class: sw-bug
>Submitter-Id: current-users
>Arrival-Date: Sat Sep 11 14:50:23 GMT 2004
>Closed-Date:
>Last-Modified:
>Originator: Dan Lukes
>Release: FreeBSD 5.3-BETA3 i386
>Organization:
Obludarium
>Environment:
System: FreeBSD 5.3-BETA3: Sun Sep 5 07:06:40 CEST 2004 i386
usr.sbin/ppp/ccp.c,v 1.76 2002/08/27 20:11:57 brian
usr.sbin/ppp/chap.c,v 1.85 2003/11/10 21:56:02 brian
usr.sbin/ppp/chat.c,v 1.78 2002/06/15 08:03:29 brian
usr.sbin/ppp/datalink.c,v 1.75 2003/03/26 02:03:08 brian
usr.sbin/ppp/nat_cmd.c,v 1.60 2003/09/23 07:41:54 marcus
usr.sbin/ppp/physical.c,v 1.56 2004/07/29 05:59:43 glebius
usr.sbin/ppp/physical.h,v 1.27 2004/07/29 05:59:43 glebius
usr.sbin/ppp/radius.c,v 1.48 2004/07/28 07:20:04 kan
usr.sbin/ppp/route.c,v 1.91 2003/03/25 16:49:08 ume
>Description:
There are too many places where programmer trust that the malloc() wouldn't return
NULL.
In advance I'm eliminate those warnings:
usr.sbin/ppp/radius.c:1000: warning: `return' with no value, in function returning non-void
usr.sbin/ppp/radius.c:851: warning: 'what' might be used uninitialized in this function
usr.sbin/ppp/chap.c:642: warning: implicit declaration of function `toupper'
usr.sbin/ppp/command.c:2301: warning: implicit declaration of function `physical_SetPPPoEnonstandard'
(the latest caused by typo in physical.h)
>How-To-Repeat:
N/A
>Fix:
*** usr.sbin/ppp/ccp.c.ORIG Thu Aug 29 14:14:18 2002
--- usr.sbin/ppp/ccp.c Sat Sep 11 16:24:59 2004
***************
*** 366,371 ****
--- 366,375 ----
if (alloc || *o == NULL) {
*o = (struct ccp_opt *)malloc(sizeof(struct ccp_opt));
+ if ( *o == NULL ) {
+ log_Printf(LogERROR, "%s: Not enought memory for CCP REQ !\n", fp->link->name);
+ break;
+ }
(*o)->val.hdr.id = algorithm[f]->id;
(*o)->val.hdr.len = 2;
(*o)->next = NULL;
*** usr.sbin/ppp/chap.c.ORIG Fri Nov 14 03:54:47 2003
--- usr.sbin/ppp/chap.c Sat Sep 11 14:05:25 2004
***************
*** 49,54 ****
--- 49,55 ----
#include <sys/wait.h>
#include <termios.h>
#include <unistd.h>
+ #include <ctype.h>
#include "layer.h"
#include "mbuf.h"
*** usr.sbin/ppp/chat.c.ORIG Sat Jun 15 10:03:29 2002
--- usr.sbin/ppp/chat.c Sat Sep 11 15:29:22 2004
***************
*** 244,251 ****
}
c->abort.string[i].len = len;
c->abort.string[i].data = (char *)malloc(len+1);
! memcpy(c->abort.string[i].data, c->exp+2, len+1);
! c->abort.num++;
} else
log_Printf(LogERROR, "chat_UpdateSet: too many abort strings\n");
gotabort = 0;
--- 244,254 ----
}
c->abort.string[i].len = len;
c->abort.string[i].data = (char *)malloc(len+1);
! if ( c->abort.string[i].data != NULL ) {
! memcpy(c->abort.string[i].data, c->exp+2, len+1);
! c->abort.num++;
! } else
! log_Printf(LogERROR, "chat_UpdateSet: not enought memory to store abort string %s\n", c->exp+2);
} else
log_Printf(LogERROR, "chat_UpdateSet: too many abort strings\n");
gotabort = 0;
*** usr.sbin/ppp/datalink.c.ORIG Sun Apr 13 20:16:10 2003
--- usr.sbin/ppp/datalink.c Sat Sep 11 16:22:44 2004
***************
*** 1307,1318 ****
/* Make sure the name is unique ! */
oname = NULL;
do {
for (cdl = bundle->links; cdl; cdl = cdl->next)
if (!strcasecmp(dl->name, cdl->name)) {
! if (oname)
! free(datalink_NextName(dl));
! else
! oname = datalink_NextName(dl);
break; /* Keep renaming 'till we have no conflicts */
}
} while (cdl);
--- 1307,1328 ----
/* Make sure the name is unique ! */
oname = NULL;
do {
+ char *pname;
+
for (cdl = bundle->links; cdl; cdl = cdl->next)
if (!strcasecmp(dl->name, cdl->name)) {
! pname=datalink_NextName(dl));
! if ( pname == NULL ) {
! log_Printf(LogERROR, "Can't create next datalink name \"%s\"\n", dl->name);
! free(dl->name);
! free(dl);
! return(dl);
! } else {
! if (oname)
! free(pname);
! else
! oname = pname;
! }
break; /* Keep renaming 'till we have no conflicts */
}
} while (cdl);
***************
*** 1432,1437 ****
--- 1442,1451 ----
n = strlen(dl->name);
name = (char *)malloc(n+3);
+ if ( name == NULL ) {
+ log_Printf(LogERROR, "datalink_NextName: No memory for NextName !\n");
+ return(NULL);
+ }
for (f = n - 1; f >= 0; f--)
if (!isdigit(dl->name[f]))
break;
*** usr.sbin/ppp/nat_cmd.c.ORIG Sat Oct 4 19:45:39 2003
--- usr.sbin/ppp/nat_cmd.c Sat Sep 11 15:22:05 2004
***************
*** 542,548 ****
case PKT_ALIAS_UNRESOLVED_FRAGMENT:
/* Save the data for later */
! fptr = malloc(bp->m_len);
bp = mbuf_Read(bp, fptr, bp->m_len);
PacketAliasSaveFragment(fptr);
log_Printf(LogDEBUG, "Store another frag (%lu) - now %d\n",
--- 542,553 ----
case PKT_ALIAS_UNRESOLVED_FRAGMENT:
/* Save the data for later */
! if ( (fptr = malloc(bp->m_len)) == NULL ) {
! log_Printf(LogWARN, "nat_LayerPull: Dropped unresolved fragment because no memory avaiable ....\n");
! m_freem(bp);
! bp = NULL;
! break;
! }
bp = mbuf_Read(bp, fptr, bp->m_len);
PacketAliasSaveFragment(fptr);
log_Printf(LogDEBUG, "Store another frag (%lu) - now %d\n",
*** usr.sbin/ppp/physical.c.ORIG Sun Aug 8 21:13:57 2004
--- usr.sbin/ppp/physical.c Sat Sep 11 15:59:15 2004
***************
*** 733,740 ****
(*h->device2iov)(h, iov, niov, maxiov, auxfd, nauxfd);
else {
iov[*niov].iov_base = malloc(sz);
! if (h)
! memcpy(iov[*niov].iov_base, h, sizeof *h);
iov[*niov].iov_len = sz;
(*niov)++;
}
--- 733,744 ----
(*h->device2iov)(h, iov, niov, maxiov, auxfd, nauxfd);
else {
iov[*niov].iov_base = malloc(sz);
! if ( iov[*niov].iov_base != NULL ) {
! if (h)
! memcpy(iov[*niov].iov_base, h, sizeof *h);
! } else {
! log_Printf(LogWARN, "physical2iov[%s]: Not enought memory!", p->link.name);
! }
iov[*niov].iov_len = sz;
(*niov)++;
}
*** usr.sbin/ppp/physical.h.ORIG Sun Aug 8 21:13:57 2004
--- usr.sbin/ppp/physical.h Sat Sep 11 14:01:32 2004
***************
*** 173,176 ****
extern void physical_SetDescriptor(struct physical *);
extern void physical_SetAsyncParams(struct physical *, u_int32_t, u_int32_t);
extern int physical_Slot(struct physical *);
! extern int physical_SetPPPoEnonstandatd(struct physical *, int);
--- 173,176 ----
extern void physical_SetDescriptor(struct physical *);
extern void physical_SetAsyncParams(struct physical *, u_int32_t, u_int32_t);
extern int physical_Slot(struct physical *);
! extern int physical_SetPPPoEnonstandard(struct physical *, int);
*** usr.sbin/ppp/radius.c.ORIG Sun Aug 8 21:13:57 2004
--- usr.sbin/ppp/radius.c Sat Sep 11 15:04:09 2004
***************
*** 213,218 ****
--- 213,224 ----
}
*buf = malloc(*len);
+ if ( *buf == NULL ) {
+ log_Printf(LogWARN, "Cannot malloc %ld bytes of memory for demangled data buffer\n",
+ (u_long)*len);
+ *len = 0;
+ return;
+ }
memcpy(*buf, P + 1, *len);
}
#endif
***************
*** 848,854 ****
struct timeval tv;
int got;
char hostname[MAXHOSTNAMELEN];
! char *mac_addr, *what;
#if 0
struct hostent *hp;
struct in_addr hostaddr;
--- 854,861 ----
struct timeval tv;
int got;
char hostname[MAXHOSTNAMELEN];
! char *mac_addr;
! char *what = what; /* to avoid "might be used unitialized" warning */
#if 0
struct hostent *hp;
struct in_addr hostaddr;
***************
*** 997,1003 ****
rad_put_string(r->cx.rad, RAD_CALLING_STATION_ID, mac_addr) != 0) {
log_Printf(LogERROR, "rad_put: %s\n", rad_strerror(r->cx.rad));
rad_close(r->cx.rad);
! return;
}
radius_put_physical_details(r->cx.rad, authp->physical);
--- 1004,1010 ----
rad_put_string(r->cx.rad, RAD_CALLING_STATION_ID, mac_addr) != 0) {
log_Printf(LogERROR, "rad_put: %s\n", rad_strerror(r->cx.rad));
rad_close(r->cx.rad);
! return 0;
}
radius_put_physical_details(r->cx.rad, authp->physical);
*** usr.sbin/ppp/route.c.ORIG Sun Apr 13 20:16:11 2003
--- usr.sbin/ppp/route.c Sat Sep 11 15:46:00 2004
***************
*** 278,287 ****
}
if (ifs[ifm->ifm_index-1] == NULL) {
ifs[ifm->ifm_index-1] = (char *)malloc(dl->sdl_nlen+1);
! memcpy(ifs[ifm->ifm_index-1], dl->sdl_data, dl->sdl_nlen);
! ifs[ifm->ifm_index-1][dl->sdl_nlen] = '\0';
! if (route_nifs < ifm->ifm_index)
! route_nifs = ifm->ifm_index;
}
} else if (log_IsKept(LogDEBUG))
log_Printf(LogDEBUG, "Skipping out-of-range interface %d!\n",
--- 278,291 ----
}
if (ifs[ifm->ifm_index-1] == NULL) {
ifs[ifm->ifm_index-1] = (char *)malloc(dl->sdl_nlen+1);
! if ( ifs[ifm->ifm_index-1] != NULL ) {
! memcpy(ifs[ifm->ifm_index-1], dl->sdl_data, dl->sdl_nlen);
! ifs[ifm->ifm_index-1][dl->sdl_nlen] = '\0';
! if (route_nifs < ifm->ifm_index)
! route_nifs = ifm->ifm_index;
! } else {
! log_Printf(LogWARN, "Index2Nam: No memory - skipping interface #%d\n", ifm->ifm_index);
! }
}
} else if (log_IsKept(LogDEBUG))
log_Printf(LogDEBUG, "Skipping out-of-range interface %d!\n",
***************
*** 614,619 ****
--- 618,627 ----
if (!r)
r = (struct sticky_route *)malloc(sizeof(struct sticky_route));
+ if (!r) {
+ log_Printf(LogERROR, "route_Add: Not enought memory!\n");
+ return;
+ }
r->type = type;
r->next = NULL;
ncprange_copy(&r->dst, dst);
>Release-Note:
>Audit-Trail:
>Unformatted:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200409111446.i8BEkKNT001836>