Another sarge upload of ekg
Hi!
Upstream has recently fixed a few bugs in libgadu in one batch. I have
split them into two patchsets.
Some of them (attached as 2005-05-24-rc-fix-onlyRC.patch) are RC in my
opinion, in that they:
- fix a DoS condition (missing check for 0 return value from read())
- fix a mistake of setting errno to 0 instead of passing appropriate
value to library user
- add input parameter checks whose lack could cause a DoS
I certainly want to make an upload with the above fixes.
The recent commit also included some other variable signedness errors,
attached as 2005-05-24-rc-fix-notRC.patch. As far as I and the upstream
author can see however, they do not enhance security, as their positive
values seem to be guaranteed by their execution environment. So if we
are right, they are not RC.
On the other hand, neither of us have much experience in doing security
audits, so we might be mistaken, therefore I would like to apply this
other patchset as well in the same upload, if only I'm allowed by the
release team.
regards,
Marcin
--
Marcin Owsiany <porridge@debian.org> http://marcin.owsiany.pl/
GnuPG: 1024D/60F41216 FE67 DA2D 0ACA FC5E 3F75 D6F6 3A0D 8AA0 60F4 1216
diff -rN -U 10 lib/common.c ../upstream/ekg/lib/common.c
--- lib/common.c 2005-03-20 01:43:44.000000000 +0100
+++ ../upstream/ekg/lib/common.c 2005-05-23 23:45:00.000000000 +0200
@@ -1,11 +1,11 @@
-/* $Id: common.c,v 1.68 2005/03/20 00:43:44 szalik Exp $ */
+/* $Id: common.c,v 1.70 2005/05/23 16:27:15 wojtekka Exp $ */
/*
* (C) Copyright 2001-2002 Wojtek Kaniewski <wojtekka@irc.pl>
* Robert J. Woźny <speedy@ziew.org>
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU Lesser General Public License Version
* 2.1 as published by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful,
@@ -287,26 +287,30 @@
/*
* gg_read_line() // funkcja pomocnicza
*
* czyta jedną linię tekstu z gniazda.
*
* - sock - deskryptor gniazda
* - buf - wskaźnik do bufora
* - length - długość bufora
*
- * jeśli trafi na błąd odczytu, zwraca NULL. inaczej zwraca buf.
+ * jeśli trafi na błąd odczytu lub podano nieprawidłowe parametry, zwraca NULL.
+ * inaczej zwraca buf.
*/
char *gg_read_line(int sock, char *buf, int length)
{
int ret;
+ if (!buf || length < 0)
+ return NULL;
+
for (; length > 1; buf++, length--) {
do {
if ((ret = read(sock, buf, 1)) == -1 && errno != EINTR) {
gg_debug(GG_DEBUG_MISC, "// gg_read_line() error on read (errno=%d, %s)\n", errno, strerror(errno));
*buf = 0;
return NULL;
} else if (ret == 0) {
gg_debug(GG_DEBUG_MISC, "// gg_read_line() eof reached\n");
*buf = 0;
return NULL;
@@ -788,20 +792,23 @@
* - buf - bufor danych
* - size - ilość danych
*
* suma kontrolna CRC32.
*/
uint32_t gg_crc32(uint32_t crc, const unsigned char *buf, int len)
{
if (!gg_crc32_initialized)
gg_crc32_make_table();
+ if (!buf || len < 0)
+ return crc;
+
crc ^= 0xffffffffL;
while (len--)
crc = (crc >> 8) ^ gg_crc32_table[(crc ^ *buf++) & 0xff];
return crc ^ 0xffffffffL;
}
/*
diff -rN -U 10 lib/libgadu.c ../upstream/ekg/lib/libgadu.c
--- lib/libgadu.c 2005-05-08 22:39:34.000000000 +0200
+++ ../upstream/ekg/lib/libgadu.c 2005-05-23 23:45:00.000000000 +0200
@@ -1,11 +1,11 @@
-/* $Id: libgadu.c,v 1.144 2005/04/12 15:39:22 szalik Exp $ */
+/* $Id: libgadu.c,v 1.146 2005/05/23 16:38:38 wojtekka Exp $ */
/*
* (C) Copyright 2001-2003 Wojtek Kaniewski <wojtekka@irc.pl>
* Robert J. Woźny <speedy@ziew.org>
* Arkadiusz Miśkiewicz <arekm@pld-linux.org>
* Tomasz Chiliński <chilek@chilan.com>
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU Lesser General Public License Version
* 2.1 as published by the Free Software Foundation.
@@ -65,21 +65,21 @@
int gg_proxy_enabled = 0;
int gg_proxy_http_only = 0;
char *gg_proxy_username = NULL;
char *gg_proxy_password = NULL;
#ifndef lint
static char rcsid[]
#ifdef __GNUC__
__attribute__ ((unused))
#endif
-= "$Id: libgadu.c,v 1.144 2005/04/12 15:39:22 szalik Exp $";
+= "$Id: libgadu.c,v 1.146 2005/05/23 16:38:38 wojtekka Exp $";
#endif
/*
* gg_libgadu_version()
*
* zwraca wersję libgadu.
*
* - brak
*
* wersja libgadu.
@@ -448,21 +448,23 @@
}
/*
* gg_recv_packet() // funkcja wewnętrzna
*
* odbiera jeden pakiet i zwraca wskaźnik do niego. pamięć po nim
* należy zwolnić za pomocą free().
*
* - sess - opis sesji
*
- * w przypadku błędu NULL, kod błędu w errno.
+ * w przypadku błędu NULL, kod błędu w errno. należy zwrócić uwagę, że gdy
+ * połączenie jest nieblokujące, a kod błędu wynosi EAGAIN, nie udało się
+ * odczytać całego pakietu i nie należy tego traktować jako błąd.
*/
void *gg_recv_packet(struct gg_session *sess)
{
struct gg_header h;
char *buf = NULL;
int ret = 0;
unsigned int offset, size = 0;
gg_debug(GG_DEBUG_FUNCTION, "** gg_recv_packet(%p);\n", sess);
@@ -479,21 +481,21 @@
sess->header_buf = NULL;
} else
sess->header_done = 0;
while (sess->header_done < sizeof(h)) {
ret = gg_read(sess, (char*) &h + sess->header_done, sizeof(h) - sess->header_done);
gg_debug(GG_DEBUG_MISC, "// gg_recv_packet() header recv(%d,%p,%d) = %d\n", sess->fd, &h + sess->header_done, sizeof(h) - sess->header_done, ret);
if (!ret) {
- errno = 0;
+ errno = ECONNRESET;
gg_debug(GG_DEBUG_MISC, "// gg_recv_packet() header recv() failed: connection broken\n");
return NULL;
}
if (ret == -1) {
if (errno == EINTR) {
gg_debug(GG_DEBUG_MISC, "// gg_recv_packet() header recv() interrupted system call, resuming\n");
continue;
}
@@ -544,25 +546,34 @@
memcpy(buf, &h, sizeof(h));
offset = 0;
size = h.length;
}
while (size > 0) {
ret = gg_read(sess, buf + sizeof(h) + offset, size);
gg_debug(GG_DEBUG_MISC, "// gg_recv_packet() body recv(%d,%p,%d) = %d\n", sess->fd, buf + sizeof(h) + offset, size, ret);
+ if (!ret) {
+ gg_debug(GG_DEBUG_MISC, "// gg_recv_packet() body recv() failed: connection broken\n");
+ errno = ECONNRESET;
+ return NULL;
+ }
if (ret > -1 && ret <= size) {
offset += ret;
size -= ret;
} else if (ret == -1) {
+ int errno2 = errno;
+
gg_debug(GG_DEBUG_MISC, "// gg_recv_packet() body recv() failed (errno=%d, %s)\n", errno, strerror(errno));
+ errno = errno2;
+
if (errno == EAGAIN) {
gg_debug(GG_DEBUG_MISC, "// gg_recv_packet() %d bytes received, %d left\n", offset, size);
sess->recv_buf = buf;
sess->recv_left = size;
sess->recv_done = offset;
return NULL;
}
if (errno != EINTR) {
free(buf);
return NULL;
diff -rN -U 10 lib/common.c ../upstream/ekg/lib/common.c
--- lib/common.c 2005-03-20 01:43:44.000000000 +0100
+++ ../upstream/ekg/lib/common.c 2005-05-23 23:45:00.000000000 +0200
@@ -353,21 +357,21 @@
*
* - str - ciąg znaków do zakodowania
*
* zaalokowany bufor, który należy później zwolnić albo NULL
* w przypadku błędu.
*/
char *gg_urlencode(const char *str)
{
char *q, *buf, hex[] = "0123456789abcdef";
const char *p;
- int size = 0;
+ unsigned int size = 0;
if (!str)
str = "";
for (p = str; *p; p++, size++) {
if (!((*p >= 'a' && *p <= 'z') || (*p >= 'A' && *p <= 'Z') || (*p >= '0' && *p <= '9') || *p == ' ') || (*p == '@') || (*p == '.') || (*p == '-'))
size += 2;
}
if (!(buf = malloc(size + 1)))
@@ -405,32 +409,32 @@
*/
int gg_http_hash(const char *format, ...)
{
unsigned int a, c, i, j;
va_list ap;
int b = -1;
va_start(ap, format);
for (j = 0; j < strlen(format); j++) {
- unsigned char *arg, buf[16];
+ char *arg, buf[16];
if (format[j] == 'u') {
snprintf(buf, sizeof(buf), "%d", va_arg(ap, uin_t));
arg = buf;
} else {
- if (!(arg = va_arg(ap, unsigned char*)))
+ if (!(arg = va_arg(ap, char*)))
arg = "";
}
i = 0;
- while ((c = (int) arg[i++]) != 0) {
+ while ((c = (unsigned char) arg[i++]) != 0) {
a = (c ^ b) + (c << 8);
b = (a >> 24) | (a << 8);
}
}
va_end(ap);
return (b < 0 ? -b : b);
}
@@ -606,21 +610,21 @@
*
* zapisuje ciąg znaków w base64.
*
* - buf - ciąg znaków.
*
* zaalokowany bufor.
*/
char *gg_base64_encode(const char *buf)
{
char *out, *res;
- int i = 0, j = 0, k = 0, len = strlen(buf);
+ unsigned int i = 0, j = 0, k = 0, len = strlen(buf);
res = out = malloc((len / 3 + 1) * 4 + 2);
if (!res)
return NULL;
while (j <= len) {
switch (i % 4) {
case 0:
k = (buf[j] & 252) >> 2;
@@ -664,21 +668,21 @@
* dekoduje ciąg znaków z base64.
*
* - buf - ciąg znaków.
*
* zaalokowany bufor.
*/
char *gg_base64_decode(const char *buf)
{
char *res, *save, *foo, val;
const char *end;
- int index = 0;
+ unsigned int index = 0;
if (!buf)
return NULL;
save = res = calloc(1, (strlen(buf) / 4 + 1) * 3 + 2);
if (!save)
return NULL;
end = buf + strlen(buf);
@@ -758,21 +762,21 @@
static uint32_t gg_crc32_table[256];
static int gg_crc32_initialized = 0;
/*
* gg_crc32_make_table() // funkcja wewnętrzna
*/
static void gg_crc32_make_table()
{
uint32_t h = 1;
- int i, j;
+ unsigned int i, j;
memset(gg_crc32_table, 0, sizeof(gg_crc32_table));
for (i = 128; i; i >>= 1) {
h = (h >> 1) ^ ((h & 1) ? 0xedb88320L : 0);
for (j = 0; j < 256; j += 2 * i)
gg_crc32_table[i + j] = gg_crc32_table[j] ^ h;
}
diff -rN -U 10 lib/dcc.c ../upstream/ekg/lib/dcc.c
--- lib/dcc.c 2005-03-20 01:43:44.000000000 +0100
+++ ../upstream/ekg/lib/dcc.c 2005-05-23 10:27:45.000000000 +0200
@@ -1,11 +1,11 @@
-/* $Id: dcc.c,v 1.68 2005/03/20 00:43:44 szalik Exp $ */
+/* $Id: dcc.c,v 1.69 2005/05/22 08:41:54 wojtekka Exp $ */
/*
* (C) Copyright 2001-2002 Wojtek Kaniewski <wojtekka@irc.pl>
* Tomasz Chiliński <chilek@chilan.com>
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU Lesser General Public License Version
* 2.1 as published by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful,
@@ -45,23 +45,23 @@
/*
* gg_dcc_debug_data() // funkcja wewnętrzna
*
* wyświetla zrzut pakietu w hexie.
*
* - prefix - prefiks zrzutu pakietu
* - fd - deskryptor gniazda
* - buf - bufor z danymi
* - size - rozmiar danych
*/
-static void gg_dcc_debug_data(const char *prefix, int fd, const void *buf, int size)
+static void gg_dcc_debug_data(const char *prefix, int fd, const void *buf, unsigned int size)
{
- int i;
+ unsigned int i;
gg_debug(GG_DEBUG_MISC, "++ gg_dcc %s (fd=%d,len=%d)", prefix, fd, size);
for (i = 0; i < size; i++)
gg_debug(GG_DEBUG_MISC, " %.2x", ((unsigned char*) buf)[i]);
gg_debug(GG_DEBUG_MISC, "\n");
}
#else
#define gg_dcc_debug_data(a,b,c,d) do { } while (0)
diff -rN -U 10 lib/events.c ../upstream/ekg/lib/events.c
--- lib/events.c 2005-05-08 22:39:33.000000000 +0200
+++ ../upstream/ekg/lib/events.c 2005-05-23 10:27:45.000000000 +0200
@@ -1,11 +1,11 @@
-/* $Id: events.c,v 1.86 2005/04/12 15:39:22 szalik Exp $ */
+/* $Id: events.c,v 1.87 2005/05/22 08:41:55 wojtekka Exp $ */
/*
* (C) Copyright 2001-2003 Wojtek Kaniewski <wojtekka@irc.pl>
* Robert J. Woźny <speedy@ziew.org>
* Arkadiusz Miśkiewicz <arekm@pld-linux.org>
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU Lesser General Public License Version
* 2.1 as published by the Free Software Foundation.
*
@@ -160,21 +160,21 @@
}
/*
* gg_image_queue_parse() // funkcja wewnętrzna
*
* parsuje przychodzący pakiet z obrazkiem.
*
* - e - opis zdarzenia
* -
*/
-static void gg_image_queue_parse(struct gg_event *e, char *p, int len, struct gg_session *sess, uin_t sender)
+static void gg_image_queue_parse(struct gg_event *e, char *p, unsigned int len, struct gg_session *sess, uin_t sender)
{
struct gg_msg_image_reply *i = (void*) p;
struct gg_image_queue *q, *qq;
if (!p || !sess || !e)
{
errno = EFAULT;
return;
}
@@ -387,21 +387,21 @@
e->event.image_reply.filename = strdup("");
e->event.image_reply.image = strdup("");
return 0;
} else if (p + sizeof(struct gg_msg_image_reply) + 1 > packet_end) {
gg_debug(GG_DEBUG_MISC, "// gg_handle_recv_msg() packet out of bounds (4)\n");
goto malformed;
}
- gg_image_queue_parse(e, p, (int)(packet_end - p), sess, gg_fix32(r->sender));
+ gg_image_queue_parse(e, p, (unsigned int)(packet_end - p), sess, gg_fix32(r->sender));
return 0;
}
default:
{
gg_debug(GG_DEBUG_MISC, "// gg_handle_recv_msg() unknown payload 0x%.2x\n", *p);
p = packet_end;
}
}
@@ -464,21 +464,21 @@
if (h->length >= sizeof(struct gg_recv_msg))
if (gg_handle_recv_msg(h, e, sess))
goto fail;
break;
}
case GG_NOTIFY_REPLY:
{
struct gg_notify_reply *n = (void*) p;
- int count, i;
+ unsigned int count, i;
char *tmp;
gg_debug(GG_DEBUG_MISC, "// gg_watch_fd_connected() received a notify reply\n");
if (h->length < sizeof(*n)) {
gg_debug(GG_DEBUG_MISC, "// gg_watch_fd_connected() incomplete packet\n");
errno = EINVAL;
goto fail;
}
@@ -713,21 +713,21 @@
* gdy otrzymano wszystkie odpowiedzi */
if (p[0] == GG_USERLIST_PUT_REPLY || p[0] == GG_USERLIST_PUT_MORE_REPLY) {
if (--sess->userlist_blocks)
break;
p[0] = GG_USERLIST_PUT_REPLY;
}
if (h->length > 1) {
char *tmp;
- int len = (sess->userlist_reply) ? strlen(sess->userlist_reply) : 0;
+ unsigned int len = (sess->userlist_reply) ? strlen(sess->userlist_reply) : 0;
gg_debug(GG_DEBUG_MISC, "userlist_reply=%p, len=%d\n", sess->userlist_reply, len);
if (!(tmp = realloc(sess->userlist_reply, len + h->length))) {
gg_debug(GG_DEBUG_MISC, "// gg_watch_fd_connected() not enough memory for userlist reply\n");
free(sess->userlist_reply);
sess->userlist_reply = NULL;
goto fail;
}
Reply to: