CVS update: samba/source/rpc_parse
Andrew Bartlett
abartlet at pcug.org.au
Wed Jul 11 19:12:43 EST 2001
Andrew Bartlett wrote:
>
> Jeremy Allison wrote:
> >
> > Jean Francois Micouleau wrote:
> > >
> > > > The RPC code is currently littered with code that does init_uni_hdr() followed
> > > > immidiatly by init_unistr2(), and often the length argument is wrong. (It was
> > > > for the code I changed, even before the charset stuff). Another bug where we
> > > > made strings AT LEAST MAX_UNICODE_LEN long hid this bug.
> > >
> > > your patch is wrong.
> > >
> > > the header length is USUALLY (with some exceptions) the number of bytes
> > > and not the number of characters.
>
> OK, I assumed some sanity where there obviously is none.
>
> > > A UNISTR2 is not always NULL terminated. On some pipes and certains rpc
> > > functions, the string must be NON NULL terminated with uni_str_len being
> > > the length of the string and uni_max_len the length plus 1.
>
> Any chance of making this clearer in the header files defining the
> structures or the functions creating them? So the next poor sod who
> thinks 'I can fix that' doesn't end up in the same mess?
>
> > > and btw, you're not checking if str and hdr ptrs are NON NULL before
> > > affecting values.
>
> Neither does the code I was replacing... (init_unistr2 and
> init_str_hdr).
>
> > Andrew, can you roll back your patch please and pass it via
> > JF for correctlness checks before committing it again.
> >
> > tridge and I are a little tied up right now so can't give
> > this the attention it deserves.
>
> I'll roll it back to a fixed for English, broken for multibyte stage,
> where assumptions about strlen()*2 prevail.
As you would have seen, this has now been rolled back. I propose the
attached patch as fix for the sane cases where the various lengths are
equal, which occours in at least some places in the code (or so I
understand), and takes an arugument as to null termination.
Is this patch sane? It would seem to me to be a good step towards
multibyte complient RPC, but as I have shown, I'm no expert in this
area. In particular I'm not sure about what is the correct behaviour
when buf == NULL.
Andrew Bartlett
--
Andrew Bartlett
abartlet at pcug.org.au
abartlet at samba.org
-------------- next part --------------
Index: rpc_parse/parse_misc.c
===================================================================
RCS file: /data/cvs/samba/source/rpc_parse/parse_misc.c,v
retrieving revision 1.84
diff -u -r1.84 parse_misc.c
--- rpc_parse/parse_misc.c 11 Jul 2001 04:27:03 -0000 1.84
+++ rpc_parse/parse_misc.c 11 Jul 2001 08:08:06 -0000
@@ -5,6 +5,7 @@
* Copyright (C) Andrew Tridgell 1992-1997,
* Copyright (C) Luke Kenneth Casson Leighton 1996-1997,
* Copyright (C) Paul Ashton 1997.
+ * Copyright (C) Andrew Bartlett 2001.
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
@@ -918,6 +919,61 @@
return;
rpcstr_push((char *)str->buffer, buf, len, STR_TERMINATE);
+}
+
+/*******************************************************************
+ Inits a UNIHDR and UNISTR2 structure at one time.
+********************************************************************/
+
+void init_unistr2_and_hdr(UNISTR2 *str, UNIHDR *hdr, const char *buf, BOOL null_terminate )
+{
+ size_t convbuf_len_bytes, len_bytes;
+ int len;
+
+ uint16 *conversion_buffer;
+
+ if ((str == NULL) || (hdr == NULL))
+ return;
+
+ if (buf == NULL) {
+ str->buffer = NULL;
+ hdr->uni_str_len = 0;
+ hdr->uni_max_len = 0;
+ hdr->buffer = 0;
+ return;
+ }
+
+ convbuf_len_bytes = (sizeof(uint16)*(strlen(buf) + 1));
+ /* Our strings cannot expand from internal to unicode by more
+ than a factor of 2 */
+
+ conversion_buffer = malloc(convbuf_len_bytes);
+ if (conversion_buffer == NULL)
+ smb_panic("init_unistr: malloc fail\n");
+
+ len_bytes = rpcstr_push(conversion_buffer, buf, convbuf_len_bytes, null_terminate ? STR_TERMINATE: 0);
+
+ len = len_bytes/sizeof(uint16);
+
+ if (len > MAX_UNISTRLEN) {
+ len = MAX_UNISTRLEN;
+ }
+
+ str->buffer = (uint16 *)talloc_zero(get_talloc_ctx(), len*sizeof(uint16));
+ if (str->buffer == NULL)
+ smb_panic("init_unistr: talloc fail\n");
+
+ hdr->uni_str_len = len;
+ hdr->uni_max_len = len;
+
+ hdr->buffer = 1;
+
+ str->uni_str_len = len;
+ str->undoc = 0;
+ str->uni_max_len = len;
+ memcpy(str->buffer, conversion_buffer, len*sizeof(uint16));
+
+ free(conversion_buffer);
}
/*******************************************************************
Index: rpc_parse/parse_net.c
===================================================================
RCS file: /data/cvs/samba/source/rpc_parse/parse_net.c,v
retrieving revision 1.57
diff -u -r1.57 parse_net.c
--- rpc_parse/parse_net.c 11 Jul 2001 04:27:03 -0000 1.57
+++ rpc_parse/parse_net.c 11 Jul 2001 08:08:12 -0000
@@ -5,6 +5,7 @@
* Copyright (C) Andrew Tridgell 1992-1997,
* Copyright (C) Luke Kenneth Casson Leighton 1996-1997,
* Copyright (C) Paul Ashton 1997.
+ * Copyright (C) Andrew Bartlett 2001.
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
@@ -983,9 +984,6 @@
unsigned char *lm_chal_resp,
unsigned char *nt_chal_resp)
{
- int len_domain_name = strlen(domain_name);
- int len_user_name = strlen(user_name );
- int len_wksta_name = strlen(wksta_name );
int nt_chal_resp_len = ((nt_chal_resp != NULL) ? 24 : 0);
int lm_chal_resp_len = ((lm_chal_resp != NULL) ? 24 : 0);
unsigned char lm_owf[24];
@@ -995,14 +993,9 @@
id->ptr_id_info2 = 1;
- init_uni_hdr(&id->hdr_domain_name, len_domain_name);
-
id->param_ctrl = param_ctrl;
init_logon_id(&id->logon_id, log_id_low, log_id_high);
- init_uni_hdr(&id->hdr_user_name, len_user_name);
- init_uni_hdr(&id->hdr_wksta_name, len_wksta_name);
-
if (nt_chal_resp) {
/* oops. can only send what-ever-it-is direct */
memcpy(nt_owf, nt_chal_resp, 24);
@@ -1018,9 +1011,9 @@
init_str_hdr(&id->hdr_nt_chal_resp, 24, nt_chal_resp_len, (nt_chal_resp != NULL) ? 1 : 0);
init_str_hdr(&id->hdr_lm_chal_resp, 24, lm_chal_resp_len, (lm_chal_resp != NULL) ? 1 : 0);
- init_unistr2(&id->uni_domain_name, domain_name, len_domain_name);
- init_unistr2(&id->uni_user_name, user_name, len_user_name);
- init_unistr2(&id->uni_wksta_name, wksta_name, len_wksta_name);
+ init_unistr2_and_hdr(&id->uni_domain_name, &id->hdr_domain_name, domain_name, False);
+ init_unistr2_and_hdr(&id->uni_user_name, &id->hdr_user_name, user_name, False);
+ init_unistr2_and_hdr(&id->uni_wksta_name, &id->hdr_wksta_name, wksta_name, False);
init_string2(&id->nt_chal_resp, (char *)nt_chal_resp, nt_chal_resp_len);
init_string2(&id->lm_chal_resp, (char *)lm_chal_resp, lm_chal_resp_len);
More information about the samba-cvs
mailing list