Updated ChangeLog

- Fixed bug #95 (DNS lookup failing on Cygwin) - Removed dependency on the check package
README: remove mention of the check dependency
2016-09-16 01:10:25 +03:00 · 2016-09-16 01:07:21 +03:00 · 2016-09-16 01:01:43 +03:00 · 2016-09-15 19:26:10 +03:00 · 2016-09-15 12:46:11 +03:00 · 2016-09-14 19:53:25 +01:00
9 changed files with 180 additions and 44 deletions
--- a/4
+++ b/4
@@ -1,3 +1,7 @@
+0.9.1
+	- Fixed bug #95 (DNS lookup failing on Cygwin)
+	- Removed dependency on the check package
+
 0.9.0
 	- IPv6 support
 	- Legacy SSL support
--- a/Makefile.am
+++ b/Makefile.am
@@ -81,9 +81,9 @@ TESTS = tests/check_parser tests/test_sha1 tests/test_md5 tests/test_rand \
 check_PROGRAMS = $(TESTS)

 tests_check_parser_SOURCES = tests/check_parser.c tests/test.h
-tests_check_parser_CFLAGS = @check_CFLAGS@ $(PARSER_CFLAGS) $(STROPHE_FLAGS) \
+tests_check_parser_CFLAGS = $(PARSER_CFLAGS) $(STROPHE_FLAGS) \
 	-I$(top_srcdir)/src
-tests_check_parser_LDADD = @check_LIBS@ $(STROPHE_LIBS)
+tests_check_parser_LDADD = $(STROPHE_LIBS)
 tests_check_parser_LDFLAGS = -static

 tests_test_ctx_SOURCES = tests/test_ctx.c
--- a/README.markdown
+++ b/README.markdown
@@ -58,9 +58,6 @@ libstrophe requires:
 To build libstrophe using autotools you will need autoconf, automake,
 libtool and pkg-config.

-In addition, if you wish to run the unit tests, you will need the
-check package.
-
 Installation
 ------------

--- a/configure.ac
+++ b/configure.ac
@@ -1,4 +1,4 @@
-AC_INIT([libstrophe], [0.9.0], [jack@metajack.im])
+AC_INIT([libstrophe], [0.9.1], [jack@metajack.im])
 AC_CONFIG_MACRO_DIR([m4])
 AM_INIT_AUTOMAKE([foreign])
 LT_INIT([dlopen])
@@ -22,9 +22,6 @@ AC_ARG_WITH([libxml2],
 AC_ARG_ENABLE([tls],
    [AS_HELP_STRING([--disable-tls], [disable TLS support])])

-PKG_CHECK_MODULES([check], [check >= 0.9.4],
-    [], [AC_MSG_WARN([libcheck not found; unit tests will not be compilable])])
-
 if test "x$enable_tls" != xno; then
    PKG_CHECK_MODULES([openssl], [openssl],
        [PC_REQUIRES="openssl ${PC_REQUIRES}"],
--- a/src/resolver.c
+++ b/src/resolver.c
@@ -23,6 +23,7 @@

 #include "ostypes.h"
 #include "snprintf.h"
+#include "util.h"               /* xmpp_min */
 #include "resolver.h"

 #define MESSAGE_HEADER_LEN 12
@@ -65,6 +66,26 @@ static uint8_t message_header_rcode(const struct message_header *header)
    return header->octet3 & 0x0f;
 }

+/*
+ * Append a label or a dot to the target name with buffer overflow checks.
+ * Returns length of the non-truncated resulting string, may be bigger than
+ * name_max.
+ */
+static size_t message_name_append_safe(char *name, size_t name_len,
+                                       size_t name_max,
+                                       const char *tail, size_t tail_len)
+{
+    size_t copy_len;
+
+    copy_len = name_max > name_len ? name_max - name_len : 0;
+    copy_len = xmpp_min(tail_len, copy_len);
+    if (copy_len > 0)
+        strncpy(&name[name_len], tail, copy_len);
+
+    return name_len + tail_len;
+}
+
+/* Returns length of the compressed name. This is NOT the same as strlen(). */
 static unsigned message_name_get(const unsigned char *buf, size_t buf_len,
                                 unsigned buf_offset,
                                 char *name, size_t name_max)
@@ -72,25 +93,41 @@ static unsigned message_name_get(const unsigned char *buf, size_t buf_len,
    size_t name_len = 0;
    unsigned i = buf_offset;
    unsigned pointer;
+    unsigned rc;
    unsigned char label_len;

-    while ((label_len = buf[i++]) != 0) {
-        /* label */
+
+    while (1) {
+        if (i >= buf_len) return 0;
+        label_len = buf[i++];
+        if (label_len == 0) break;
+
+        /* Label */
        if ((label_len & 0xc0) == 0) {
+            if (i + label_len - 1 >= buf_len) return 0;
            if (name != NULL) {
-                if (name_len != 0)
-                    name[name_len++] = '.';
-                strncpy(&name[name_len], (char *)&buf[i], label_len);
+                name_len = message_name_append_safe(name, name_len, name_max,
+                                                    (char *)&buf[i], label_len);
+                name_len = message_name_append_safe(name, name_len, name_max,
+                                                    ".", 1);
            }
            i += label_len;
-            name_len += label_len;

-        /* pointer */
+        /* Pointer */
        } else if ((label_len & 0xc0) == 0xc0) {
+            if (i >= buf_len) return 0;
            pointer = (label_len & 0x3f) << 8 | buf[i++];
-            (void)message_name_get(buf, buf_len, pointer, &name[name_len],
-                                   name_max - name_len);
-            /* pointer is always the last */
+            if (name != NULL && name_len >= name_max && name_max > 0) {
+                /* We have filled the name buffer. Don't pass it recursively. */
+                name[name_max - 1] = '\0';
+                name = NULL;
+                name_max = 0;
+            }
+            rc = message_name_get(buf, buf_len, pointer,
+                                  name != NULL ? &name[name_len] : NULL,
+                                  name_max > name_len ? name_max - name_len : 0);
+            if (rc == 0) return 0;
+            /* Pointer is always the last. */
            break;

        /* The 10 and 01 combinations are reserved for future use. */
@@ -98,8 +135,21 @@ static unsigned message_name_get(const unsigned char *buf, size_t buf_len,
            return 0;
        }
    }
-    if (label_len == 0 && name != NULL)
-        name[name_len] = '\0';
+    if (label_len == 0) {
+        if (name_len == 0) name_len = 1;
+        /*
+         * At this point name_len is length of the resulting name,
+         * including '\0'. This value can be exported to allocate buffer
+         * of precise size.
+         */
+        if (name != NULL && name_max > 0) {
+            /*
+             * Overwrite leading '.' with a '\0'. If the resulting name is
+             * bigger than name_max it is truncated.
+             */
+            name[xmpp_min(name_len, name_max) - 1] = '\0';
+        }
+    }

    return i - buf_offset;
 }
@@ -167,6 +217,15 @@ static void resolver_srv_list_sort(resolver_srv_rr_t **srv_rr_list)
    *srv_rr_list = rr_head;
 }

+#define BUF_OVERFLOW_CHECK(ptr, len) do {         \
+    if ((ptr) >= (len)) {                         \
+        if (*srv_rr_list != NULL)                 \
+            resolver_srv_free(ctx, *srv_rr_list); \
+        *srv_rr_list = NULL;                      \
+        return XMPP_DOMAIN_NOT_FOUND;             \
+    }                                             \
+} while (0)
+
 int resolver_srv_lookup_buf(xmpp_ctx_t *ctx, const unsigned char *buf,
                            size_t len, resolver_srv_rr_t **srv_rr_list)
 {
@@ -200,17 +259,20 @@ int resolver_srv_lookup_buf(xmpp_ctx_t *ctx, const unsigned char *buf,

    /* skip question section */
    for (i = 0; i < header.qdcount; ++i) {
+        BUF_OVERFLOW_CHECK(j, len);
        name_len = message_name_len(buf, len, j);
-        if (name_len == 0) {
-            /* error in name format */
-            return XMPP_DOMAIN_NOT_FOUND;
-        }
+        /* error in name format */
+        if (name_len == 0) return XMPP_DOMAIN_NOT_FOUND;
        j += name_len + 4;
    }

    for (i = 0; i < header.ancount; ++i) {
+        BUF_OVERFLOW_CHECK(j, len);
        name_len = message_name_len(buf, len, j);
+        /* error in name format */
+        if (name_len == 0) return XMPP_DOMAIN_NOT_FOUND;
        j += name_len;
+        BUF_OVERFLOW_CHECK(j + 16, len);
        type = xmpp_ntohs_ptr(&buf[j]);
        class = xmpp_ntohs_ptr(&buf[j + 2]);
        rdlength = xmpp_ntohs_ptr(&buf[j + 8]);
--- a/src/resolver.h
+++ b/src/resolver.h
@@ -65,4 +65,5 @@ int resolver_srv_lookup(xmpp_ctx_t *ctx, const char *service, const char *proto,
 *  @param srv_rr_list a list allocated by lookup functions
 */
 void resolver_srv_free(xmpp_ctx_t *ctx, resolver_srv_rr_t *srv_rr_list);
+
 #endif /* __LIBSTROPHE_RESOLVER_H__ */
--- a/src/util.h
+++ b/src/util.h
@@ -18,6 +18,9 @@

 #include "ostypes.h"

+/* TODO evaluate x and y only once */
+#define xmpp_min(x, y) ((x) < (y) ? (x) : (y))
+
 /* string functions */
 char *xmpp_strtok_r(char *s, const char *delim, char **saveptr);

--- a/tests/check_parser.c
+++ b/tests/check_parser.c
@@ -10,27 +10,34 @@
 */

 #include <stdlib.h>
+#include <stdio.h>
 #include <string.h>

-#include <check.h>
-
-#include <strophe.h>
+#include "strophe.h"
 #include "parser.h"

 #include "test.h"

-START_TEST(create_destroy)
+#define fail_unless(expr) do {                  \
+    int result = (expr);                        \
+    if (!result) {                              \
+        printf("%s:%d: Assertion failed: %s\n", \
+               __FILE__, __LINE__, #expr);      \
+        exit(1);                                \
+    }                                           \
+} while (0)
+
+static void create_destroy(void)
 {
    xmpp_ctx_t *ctx;
    parser_t *parser;

    ctx = xmpp_ctx_new(NULL, NULL);
    parser = parser_new(ctx, NULL, NULL, NULL, NULL);
-    fail_unless(parser != NULL, "Parser creation failed.");
+    fail_unless(parser != NULL);
    parser_free(parser);
    xmpp_ctx_free(ctx);
 }
-END_TEST

 int cbtest_got_start = 0;
 void cbtest_handle_start(char *name, char **attrs, void *userdata)
@@ -53,7 +60,7 @@ void cbtest_handle_stanza(xmpp_stanza_t *stanza, void *userdata)
        cbtest_got_stanza = 1;
 }

-START_TEST(callbacks)
+static void callbacks(void)
 {
    xmpp_ctx_t *ctx;
    parser_t *parser;
@@ -79,16 +86,18 @@ START_TEST(callbacks)
    parser_free(parser);
    xmpp_ctx_free(ctx);
 }
-END_TEST

-Suite *parser_suite(void)
+int main()
 {
-    Suite *s = suite_create("Parser");
-    TCase *tc_core = tcase_create("Core");
-    tcase_add_test(tc_core, create_destroy);
-    tcase_add_test(tc_core, callbacks);
-    suite_add_tcase(s, tc_core);
-    return s;
-}
+    printf("XML parser tests.\n");

-TEST_MAIN
+    printf("create-destroy: ");
+    create_destroy();
+    printf("ok\n");
+
+    printf("callbacks: ");
+    callbacks();
+    printf("ok\n");
+
+    return 0;
+}
--- a/tests/test_resolver.c
+++ b/tests/test_resolver.c
@@ -13,6 +13,7 @@
 #include <stdio.h>

 #include "strophe.h"
+#include "rand.h"
 #include "resolver.h"
 #include "test.h"

@@ -84,7 +85,6 @@ static const unsigned char data3[] = {
    0x6f, 0x6f, 0x67, 0x6c, 0x65, 0x03, 0x63, 0x6f,
    0x6d, 0x00,
 };
-
 /* res_query("_xmpp-client._tcp.jabber.calyxinstitute.org", C_IN, T_SRV, ...) */
 static const unsigned char data4[] = {
    0x8d, 0x58, 0x81, 0x80, 0x00, 0x01, 0x00, 0x02, 0x00, 0x00,    // .X........
@@ -103,6 +103,34 @@ static const unsigned char data4[] = {
    0x61, 0x6c, 0x79, 0x78, 0x69, 0x6e, 0x73, 0x74, 0x69, 0x74,    // alyxinstit
    0x75, 0x74, 0x65, 0x03, 0x6f, 0x72, 0x67, 0x00,                // ute.org.
 };
+/* res_query("_xmpp-client._tcp.jabber.org", C_IN, T_SRV, ...) with pointers */
+static const unsigned char data5[] = {
+    0x00, 0x00, 0x81, 0x80, 0x00, 0x01, 0x00, 0x02, 0x00, 0x00,    // ..........
+    0x00, 0x00, 0x0c, 0x5f, 0x78, 0x6d, 0x70, 0x70, 0x2d, 0x63,    // ..._xmpp-c
+    0x6c, 0x69, 0x65, 0x6e, 0x74, 0x04, 0x5f, 0x74, 0x63, 0x70,    // lient._tcp
+    0x06, 0x6a, 0x61, 0x62, 0x62, 0x65, 0x72, 0x03, 0x6f, 0x72,    // .jabber.or
+    0x67, 0x00, 0x00, 0x21, 0x00, 0x01, 0xc0, 0x0c, 0x00, 0x21,    // g..!.....!
+    0x00, 0x01, 0x00, 0x00, 0x01, 0x9e, 0x00, 0x12, 0x00, 0x1f,    // ..........
+    0x00, 0x1e, 0x14, 0x66, 0x09, 0x68, 0x65, 0x72, 0x6d, 0x65,    // ...f.herme
+    0x73, 0x32, 0x76, 0x36, 0xc0, 0x1e, 0xc0, 0x0c, 0x00, 0x21,    // s2v6.....!
+    0x00, 0x01, 0x00, 0x00, 0x01, 0x9e, 0x00, 0x10, 0x00, 0x1e,    // ..........
+    0x00, 0x1e, 0x14, 0x66, 0x07, 0x68, 0x65, 0x72, 0x6d, 0x65,    // ...f.herme
+    0x73, 0x32, 0xc0, 0x1e,                                        // s2..
+};
+/* hacked data2 with two empty-string targets. */
+static const unsigned char data6[] = {
+    0xf2, 0x98, 0x81, 0x80, 0x00, 0x01, 0x00, 0x02,
+    0x00, 0x00, 0x00, 0x00, 0x0c, 0x5f, 0x78, 0x6d,
+    0x70, 0x70, 0x2d, 0x63, 0x6c, 0x69, 0x65, 0x6e,
+    0x74, 0x04, 0x5f, 0x74, 0x63, 0x70, 0x06, 0x6a,
+    0x61, 0x62, 0x62, 0x65, 0x72, 0x03, 0x6f, 0x72,
+    0x67, 0x00, 0x00, 0x21, 0x00, 0x01, 0xc0, 0x0c,
+    0x00, 0x21, 0x00, 0x01, 0x00, 0x00, 0x03, 0x83,
+    0x00, 0x07, 0x00, 0x1e, 0x00, 0x1e, 0x14, 0x66,
+    0x00, 0xc0, 0x0c, 0x00, 0x21, 0x00, 0x01, 0x00,
+    0x00, 0x03, 0x83, 0x00, 0x08, 0x00, 0x1f, 0x00,
+    0x1e, 0x14, 0x66, 0xc0, 0x40,
+};

 static const struct {
    const unsigned char *data;
@@ -139,13 +167,27 @@ static const struct {
        .port = 5222,
        .target_nr = 2,
    },
+    {
+        .data = data5,
+        .len = sizeof(data5),
+        .target = "hermes2.jabber.org",
+        .port = 5222,
+        .target_nr = 2,
+    },
+    {
+        .data = data6,
+        .len = sizeof(data6),
+        .target = "",
+        .port = 5222,
+        .target_nr = 2,
+    },
 };

 static int srv_rr_list_len(resolver_srv_rr_t *list)
 {
    int nr;

-    for(nr = 0; list != NULL; ++nr, list = list->next);
+    for (nr = 0; list != NULL; ++nr, list = list->next);

    return nr;
 }
@@ -153,8 +195,10 @@ static int srv_rr_list_len(resolver_srv_rr_t *list)
 int main(int argc, char **argv)
 {
    xmpp_ctx_t *ctx;
+    xmpp_rand_t *rand;
    resolver_srv_rr_t *srv_rr_list;
    char *domain;
+    unsigned char *buf;
    unsigned short port;
    size_t i;
    int ret;
@@ -187,6 +231,25 @@ int main(int argc, char **argv)
        resolver_srv_free(ctx, srv_rr_list);
    }

+    /*
+     * The next test case must not crash and is supposed to be checked
+     * under valgrind.
+     */
+    printf("Test of a broken message: ");
+    rand = xmpp_rand_new(ctx);
+    assert(rand != NULL);
+    assert(sizeof(data2) > 64);
+    buf = xmpp_alloc(ctx, sizeof(data2));
+    assert(buf != NULL);
+    memcpy(buf, data2, 64);
+    xmpp_rand_bytes(rand, &buf[64], sizeof(data2) - 64);
+    ret = resolver_srv_lookup_buf(ctx, buf, sizeof(data2), &srv_rr_list);
+    if (ret == XMPP_DOMAIN_FOUND && srv_rr_list != NULL)
+        resolver_srv_free(ctx, srv_rr_list);
+    xmpp_free(ctx, buf);
+    xmpp_rand_free(ctx, rand);
+    printf("ok\n");
+
    xmpp_ctx_free(ctx);

    return 0;
Author	SHA1	Message	Date
Dmitry Podgorny	9931ad4fa2	Updated ChangeLog - Fixed bug #95 (DNS lookup failing on Cygwin) - Removed dependency on the check package	2016-09-16 01:10:25 +03:00
Dmitry Podgorny	5a1f282292	README: remove mention of the check dependency	2016-09-16 01:07:21 +03:00
Dmitry Podgorny	b360316b8b	tests/resolver: added test case with pointers	2016-09-16 01:01:43 +03:00
Dmitry Podgorny	c6aaa96dcb	resolver: fixed few bugs Fixed a bug which caused resolver to return a wrong target name with a missed ".". The bug was in the part of message_name_get() where a pointer is handled. Added checks for buffer overflow. Added 2 test cases to check a corner case and error handling.	2016-09-15 19:26:10 +03:00
Dmitry Podgorny	f0706f56c4	tests: remove dependency on check package Only check_parser depends on the check. Convert it. All tests will be rewritten at once when new testsuite is implemented.	2016-09-15 12:46:11 +03:00
James Booth	4ef2740c36	Update version to 0.9.1	2016-09-14 19:53:25 +01:00