/atheme

Permalink
Browse files

Do not copy more bytes than were allocated

  • Loading branch information...
@hce @maxteufel
hce authored and maxteufel committed Jan 9, 2016
1 parent 2d4af7a commit 87580d767868360d2fed503980129504da84b63e
Unified Split
Showing with 2 additions and 1 deletion.
  1. +2 −1 modules/transport/xmlrpc/xmlrpclib.c
View
3 modules/transport/xmlrpc/xmlrpclib.c
@@ -777,8 +777,9 @@ void xmlrpc_char_encode(char *outbuffer, const char *s1)
s->append_char(s, c);
}
}
+ s->append_char(s, 0);
- memcpy(outbuffer, s->str, XMLRPC_BUFSIZE);
+ strncpy(outbuffer, s->str, XMLRPC_BUFSIZE);

This comment has been minimized.

Show comment
Hide comment
@kaniini

kaniini Feb 2, 2018

Contributor

this is wrong.

should be something like:

*((char *) mempcpy (outbuffer, s->str, s->pos)) = '\0';

do not just change it to strlcpy, that is wrong too

also, you do not need the s->append_char(s, 0) that is just madness, mowgli_string_t is meant to be a Pascal string, so use the mempcpy above to convert it correctly
@kaniini

kaniini Feb 2, 2018 •
edited
Edited 3 times
  • @kaniini kaniini edited Feb 2, 2018 (most recent)
  • @kaniini kaniini edited Feb 2, 2018
  • @kaniini kaniini edited Feb 2, 2018

Contributor

this is wrong.

should be something like:

*((char *) mempcpy (outbuffer, s->str, s->pos)) = '\0';

do not just change it to strlcpy, that is wrong too

also, you do not need the s->append_char(s, 0) that is just madness, mowgli_string_t is meant to be a Pascal string, so use the mempcpy above to convert it correctly

This comment has been minimized.

Show comment
Hide comment
@kaniini

kaniini Feb 2, 2018

Contributor

to illustrate why the append_char is entirely unnecessary, consider:

mowgli_string_t *s = mowgli_string_create();
s->append(s, "moo");

/* contents are {3, {'m', 'o', 'o'}} */
s->append_char(s, 0);

/* contents now are {4, {'m', 'o', 'o', '\x0'}} */
char outbuffer[512];

/* outbuffer is {'m', 'o', 'o', '\x0', UNINITIALIZED}, but this is only because of the append_char */
memcpy(outbuffer, s->str, XMLRPC_BUFSIZE);

/* outbuffer is "moo", but you wasted CPU cycles for no reason */
strncpy(outbuffer, s->str, XMLRPC_BUFSIZE);

mowgli_string_reset(s);
s->append("moo");
/* now back to {3, {'m', 'o', 'o'}} */

*((char *) mempcpy (outbuffer, s->str, s->pos)) = '\0';
/* mempcpy leaves buffer as {'m', 'o', 'o', UNINITIALIZED}, but returns a pointer to the first uninitialized byte, which gets set to 0 */
@kaniini

kaniini Feb 2, 2018 •
edited
Edited 1 time
  • @kaniini kaniini edited Feb 2, 2018 (most recent)

Contributor

to illustrate why the append_char is entirely unnecessary, consider:

mowgli_string_t *s = mowgli_string_create();
s->append(s, "moo");

/* contents are {3, {'m', 'o', 'o'}} */
s->append_char(s, 0);

/* contents now are {4, {'m', 'o', 'o', '\x0'}} */
char outbuffer[512];

/* outbuffer is {'m', 'o', 'o', '\x0', UNINITIALIZED}, but this is only because of the append_char */
memcpy(outbuffer, s->str, XMLRPC_BUFSIZE);

/* outbuffer is "moo", but you wasted CPU cycles for no reason */
strncpy(outbuffer, s->str, XMLRPC_BUFSIZE);

mowgli_string_reset(s);
s->append("moo");
/* now back to {3, {'m', 'o', 'o'}} */

*((char *) mempcpy (outbuffer, s->str, s->pos)) = '\0';
/* mempcpy leaves buffer as {'m', 'o', 'o', UNINITIALIZED}, but returns a pointer to the first uninitialized byte, which gets set to 0 */
}
static void xmlrpc_append_char_encode(mowgli_string_t *s, const char *s1)

0 comments on commit 87580d7

Please sign in to comment.