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: