In 64-bit architectures (seen with x86_64), where sizeof(void *) is larger than sizeof(int), the recovery of os_type_BOOLEAN and os_type_INTEGER types through os_object_iter_get() assign sizeof(int) bytes pointed to by the parameter void **val. However, since the pointed to variable is sizeof(void *), this results in the high bytes of the variable not being set and therefore left with garbage values. In the database storage drivers, the values are then inspected as a void * cast to long int, or plain long, which access the bogus upper bytes of the value. The final effect is that the database driver randomly stores a very large integer value to the database (if saving an os_type_INTEGER), or else end up storing a TRUE value when the higher code meant a FALSE value (if saving an os_type_BOOLEAN).

Issue #48 <https://github.com/jabberd2/jabberd2/issues/48> is an instance of this bug, but the fix was committed only for the postgresql driver, and only for the os_type_INTEGER type. The mysql and sqlite drivers are still affected, as well as os_type_BOOLEAN for postgresql.

The attached patch fixes the issue for me. It has been tested for mysql, 
compile-tested only for other storages.



diff -ur jabberd-2.3.2-bak/storage/storage_mysql.c jabberd-2.3.2/storage/storage_mysql.c
--- jabberd-2.3.2-bak/storage/storage_mysql.c	2014-07-23 09:57:17.603740035 -0500
+++ jabberd-2.3.2/storage/storage_mysql.c	2014-07-23 11:26:16.840737235 -0500
@@ -194,16 +194,24 @@
             o = os_iter_object(os);
             if(os_object_iter_first(o))
                 do {
+                    /* For os_type_BOOLEAN and os_type_INTEGER, sizeof(int) bytes
+                       are stored in val, which might be less than sizeof(void *).
+                       Therefore, the difference is garbage unless cleared first.
+                     */
+                    val = NULL;
                     os_object_iter_get(o, &key, &val, &ot);
         
                     switch(ot) {
                         case os_type_BOOLEAN:
-                            cval = val ? strdup("1") : strdup("0");
+                            /* Only sizeof(int) bytes must be checked for value */
+                            cval = *((int *)(&val)) ? strdup("1") : strdup("0");
                             break;
         
                         case os_type_INTEGER:
                             cval = (char *) malloc(sizeof(char) * 20);
-                            sprintf(cval, "%ld", (long int) val);
+
+                            /* Only sizeof(int) bytes must be checked for value */
+                            sprintf(cval, "%d", *((int *)(&val)));
                             break;
         
                         case os_type_STRING:
@@ -218,7 +226,7 @@
                             strncpy(cval, "NAD", 3);
                             break;
 
-			case os_type_UNKNOWN:
+                        case os_type_UNKNOWN:
                             break;
                     }
         
diff -ur jabberd-2.3.2-bak/storage/storage_oracle.c jabberd-2.3.2/storage/storage_oracle.c
--- jabberd-2.3.2-bak/storage/storage_oracle.c	2014-07-23 09:57:17.609740036 -0500
+++ jabberd-2.3.2/storage/storage_oracle.c	2014-07-23 11:38:38.416736848 -0500
@@ -388,18 +388,26 @@
       {
         do 
         {
+          /* For os_type_BOOLEAN and os_type_INTEGER, sizeof(int) bytes
+             are stored in val, which might be less than sizeof(dvoid *).
+             Therefore, the difference is garbage unless cleared first.
+           */
+          val = NULL;
           os_object_iter_get(o, &key, &val, &ot);
 
           switch(ot) 
           {
             case os_type_BOOLEAN:
-              cval = val ? strdup("1") : strdup("0");
+              /* Only sizeof(int) bytes must be checked for value */
+              cval = *((int *)(&val)) ? strdup("1") : strdup("0");
               vlen = 1;
               break;
 
             case os_type_INTEGER:
               cval = (char *) malloc(sizeof(char) * 20);
-              sprintf(cval, "%d", (int) val);
+              
+              /* Only sizeof(int) bytes must be checked for value */
+              sprintf(cval, "%d", *((int *)(&val)));
               vlen = strlen(cval);
               break;
 
diff -ur jabberd-2.3.2-bak/storage/storage_pgsql.c jabberd-2.3.2/storage/storage_pgsql.c
--- jabberd-2.3.2-bak/storage/storage_pgsql.c	2014-07-23 09:57:17.607740036 -0500
+++ jabberd-2.3.2/storage/storage_pgsql.c	2014-07-23 11:26:18.734737234 -0500
@@ -194,16 +194,24 @@
             o = os_iter_object(os);
             if(os_object_iter_first(o))
                 do {
+                    /* For os_type_BOOLEAN and os_type_INTEGER, sizeof(int) bytes
+                       are stored in val, which might be less than sizeof(void *).
+                       Therefore, the difference is garbage unless cleared first.
+                     */
+                    val = NULL;
                     os_object_iter_get(o, &key, &val, &ot);
 
                     switch(ot) {
                         case os_type_BOOLEAN:
-                            cval = val ? strdup("t") : strdup("f");
+                            /* Only sizeof(int) bytes must be checked for value */
+                            cval = *((int *)(&val)) ? strdup("t") : strdup("f");
                             break;
 
                         case os_type_INTEGER:
                             cval = (char *) malloc(sizeof(char) * 20);
-                            sprintf(cval, "%ld", (int) (intptr_t) val);
+
+                            /* Only sizeof(int) bytes must be checked for value */
+                            sprintf(cval, "%d", *((int *)(&val)));
                             break;
 
                         case os_type_STRING:
diff -ur jabberd-2.3.2-bak/storage/storage_sqlite.c jabberd-2.3.2/storage/storage_sqlite.c
--- jabberd-2.3.2-bak/storage/storage_sqlite.c	2014-07-23 09:57:17.602740035 -0500
+++ jabberd-2.3.2/storage/storage_sqlite.c	2014-07-23 11:41:02.303736773 -0500
@@ -292,15 +292,22 @@
 	    o = os_iter_object (os);
 	    if (os_object_iter_first(o))
 		do {
+		    /* For os_type_BOOLEAN and os_type_INTEGER, sizeof(int) bytes
+		       are stored in val, which might be less than sizeof(void *).
+		       Therefore, the difference is garbage unless cleared first.
+		     */
+		    val = NULL;
 		    os_object_iter_get (o, &key, &val, &ot);
 
 		    switch(ot) {
 		     case os_type_BOOLEAN:
-		      sqlite3_bind_int (stmt, i + 2, val ? 1 : 0);
+		      /* Only sizeof(int) bytes must be checked for value */
+		      sqlite3_bind_int (stmt, i + 2, *((int *)(&val)) ? 1 : 0);
 		      break;
 
 		     case os_type_INTEGER:
-		      sqlite3_bind_int (stmt, i + 2, (long)val); // HACK ugly hack for pointer-to-int-cast
+		      /* Only sizeof(int) bytes must be checked for value */
+		      sqlite3_bind_int (stmt, i + 2, *((int *)(&val)));
 		      break;
 
 		     case os_type_STRING:

Reply via email to