Date: Fri, 30 Aug 1996 17:06:32 -0700 From: pius@ienet.com To: ascend-users@bungi.com, freebsd-isp@FreeBSD.ORG Subject: 2 bugs in Ascend's RADIUS server Message-ID: <199608310006.RAA05672@iago.ienet.com>
next in thread | raw e-mail | index | archive | help
There seem to be 2 bugs in Ascend's RADIUS server (radius-960528).
I'm not sure whether they're known bugs or not and I'm a little bit
skeptical that I've not missed something because it seems like other
people should have noticed these bugs as well.
The first one is in radiusd.c. On my FreeBSD 2.2-960612-SNAP system
it causes "Malloc warning: free(): already free chunk" messages to
be displayed on the console. The following patch fixes the problem:
*** ascend/radiusd.c Wed Jun 26 11:58:43 1996
--- freebsd/radiusd.c Fri Aug 30 13:26:39 1996
***************
*** 1719,1725 ****
authCleanup(authInfo)
AuthInfo *authInfo;
{
! pairfree(authInfo->cutList);
authInfo->cutList = (VALUE_PAIR *)0;
pairfree(authInfo->authreq->request);
memset(authInfo->authreq, 0, sizeof(AUTH_REQ));
--- 1719,1726 ----
authCleanup(authInfo)
AuthInfo *authInfo;
{
! if (authInfo->cutList != authInfo->authreq->request)
! pairfree(authInfo->cutList);
authInfo->cutList = (VALUE_PAIR *)0;
pairfree(authInfo->authreq->request);
memset(authInfo->authreq, 0, sizeof(AUTH_REQ));
Here's the hopefully not too confusing explanation:
As one can see, in the original version, if authInfo->cutList is the
same as authInfo->authreq->request, then pairfree() is called twice with
the same address which explains why already free()'d chunks are being
free()'d again (causing malloc.c in libc to produce the above warning).
cutList and authreq->request both point to the head of a linked list.
cutList points to a sublist that is cut out of authreq->request by the
cut_attribute() function. The relevant bit of code is at the beginning
of rad_authenticate():
authList = cut_attribute(authreq->request, PW_USER_NAME, &authName);
if((authName == (VALUE_PAIR *)NULL) ||
((int)strlen(authName->strvalue) <= 0)) {
log_err("Authenticate: from %s - No User Name\n",
ip_hostname(authreq->ipaddr));
authCleanup(&authInfo);
return -1;
}
else {
authInfo.authName = authName;
authInfo.cutList = authName;
cutCur = authName;
}
Now cutList will point to the same place as authreq->request only if the
User-Name attribute is the first element in the original attribute list
(pointed to by authreq->request) but this is apparently always the case.
The reason that they'll point to the same place lies in cut_attribute():
VALUE_PAIR *
cut_attribute(list, attributeId, attribute)
VALUE_PAIR *list;
int attributeId;
VALUE_PAIR **attribute;
{
VALUE_PAIR *head = (VALUE_PAIR *)0;
*attribute = (VALUE_PAIR *)0;
if( !list ) {
head = list;
}
else if( list->attribute == attributeId ) {
*attribute = list;
head = list->next;
}
else {
/* ... */
}
return head;
}
When cut_attribute returns, the third argument to cut_attribute will point
to the element that's being cut out (unless it's not in the list, of course).
The first argument to cut_attribute is never changed. Now if the attribute
that's being cut out happens to be the first element in the list
(list->attribute == attributeId), then the third argument to cut_attribute
will end up pointing to the same place the first one is pointing to.
In the first call to cut_attribute in rad_authenticate, authreq->request is
the first argument to cut_attribute and is not changed. When cut_attribute
returns, authName will point to the first element of the list (the same place
authreq->request is pointing to) because User-Name is the first attribute in
the list, and finally cutList is assigned the value of authName. Before
rad_authenticate returns, authCleanup is called and we have this problem.
The second bug is present only in radius-960528 and not in the earlier
radius-960327 (the first bug is present in both). The symptom (on my system)
is that the request-handling process segfaults and dumps core whenever a
username isn't found in the users file. This is reproducible, for example,
when I do an "Upd Rmt Cfg" from our MAX 4000 because the MAX requests profiles
for pseudo-users such as route-2, banner, etc. which are not in the users
file. Here's a patch to users.c that'll fix the problem:
*** ascend/users.c Fri May 24 13:21:09 1996
--- freebsd/users.c Fri Aug 30 16:10:37 1996
***************
*** 229,234 ****
--- 229,235 ----
userinfo_close(userfd);
return 0;
}
+ userinfo_close(userfd);
#else /* DBM_MODE */
curParseLine = 0;
while( (status = user_read(&userfd, curname, line)) == 0 ) {
***************
*** 242,248 ****
#endif /* DBM_MODE */
fprintf(errf, "%s:users %s and %s not found\n",
progname, name, "DEFAULT");
- userinfo_close(userfd);
return NO_USER_OR_DEFAULT_NAME;
}
--- 243,248 ----
This one is much easier to explain:
First of all, from looking at the code it appears that this bug only happens
when not compiling in DBM mode. I've never actually compiled in DBM mode and
used a DBM users file; so I'm not really 100% sure, but the above patch should
work for both the flat-file and the DBM version of radiusd.
The problem could be fixed in either usr_read.c or in users.c. The problem is
that the user_read() function in usr_read.c closes the file pointed to by
userfd (its first argument) and sets userfd to NULL when the end of the file
is reached. Then and only then does user_read() return a non-zero value. All
other returns from user_read() return 0.
This means, that, in flat-file mode, when the while loop returns, immediately
before the "users not found" error message is printed, userfd is already NULL
and the file pointed to by userfd is already closed, making the call to
userinfo_close() deadly because the subsequent fclose(NULL) will trigger the
segfault.
The above patch just moves the call to userinfo_close() into the DBM-specific
part of the code since the DBM file still needs to be closed since user_read()
is never called.
Hope that makes sense.
Pius
--
I N T E R N E T Pius Fischer, Chief Engineer
E X P R E S S 611 W. Sixth St., Suite 3201
N E T W O R K Los Angeles, CA 90017
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199608310006.RAA05672>