View Issue Details

IDProjectCategoryView StatusLast Update
0003235SOGoActiveSyncpublic2015-07-22 14:01
Reporterzj5121 Assigned Toludovic  
PrioritynormalSeveritymajorReproducibilityhave not tried
Status resolvedResolutionfixed 
Platform[Server] LinuxOSUbuntuOS Version14.04 LTS
Product Version2.3.0 
Fixed in Version2.3.1 
Summary0003235: a programming error in SOGoMailObject+ActiveSync.m
Description

struct GlobalObjectId {
.....
uint8_t* Data;
};

the Data is a pointer to another mem block, while in function:

  • (NSData ) _computeGlobalObjectIdFromEvent: (iCalEvent ) event
    {
    ...
    globalObjectId = [[NSData alloc] initWithBytes: &newGlobalId length: 40 + newGlobalId.Size*sizeof(uint8_t)];

..
}

newGlobalId is expected to be a flat continuous vector, that means the mem pointed by Data is not copied! instead some garbage is in.

This causes globalObjectId is wrong.

In my opinion, the struct GlobalObjectId should be:
struct GlobalObjectId {
...
uint8_t Data[];
};

and later:
newGlobalId = (struct GlobalObjectId*)calloc(sizeof(uint8_t), sizeof(struct GlobalObjectId) + 0x0c + [uidAsASCII length]);

This should be the right way.

Regards.

TagsNo tags attached.

Activities

ludovic

ludovic

2015-06-09 14:19

administrator   ~0008606

Can you provide a tested patch?

zj5121

zj5121

2015-06-10 19:52

reporter  

patch.diff (2,479 bytes)   
diff --git a/ActiveSync/SOGoMailObject+ActiveSync.m b/ActiveSync/SOGoMailObject+ActiveSync.m
index d3095a9..73653d7 100644
--- a/ActiveSync/SOGoMailObject+ActiveSync.m
+++ b/ActiveSync/SOGoMailObject+ActiveSync.m
@@ -91,7 +91,7 @@ struct GlobalObjectId {
   FILETIME                  CreationTime;
   uint8_t                   X[8];
   uint32_t                  Size;
-  uint8_t*                  Data;
+  uint8_t Data[0];
 };
 
 @implementation SOGoMailObject (ActiveSync)
@@ -123,30 +123,34 @@ struct GlobalObjectId {
   NSData *binPrefix, *globalObjectId, *uidAsASCII;
   NSString *prefix, *uid;
 
-  struct GlobalObjectId newGlobalId;
+  struct GlobalObjectId *newGlobalId;
   const char *bytes;
-  
-  prefix = @"040000008200e00074c5b7101a82e008";
 
   // dataPrefix is "vCal-Uid %x01 %x00 %x00 %x00"
-  uint8_t dataPrefix[] = { 0x76, 0x43, 0x61, 0x6c, 0x2d, 0x55, 0x69, 0x64, 0x01, 0x00, 0x00, 0x00 };
   uid = [event uid];
+  uidAsASCII = [uid dataUsingEncoding: NSASCIIStringEncoding];
+  newGlobalId = (struct GlobalObjectId *)calloc(
+          sizeof(uint8_t),
+          sizeof(struct GlobalObjectId) + 0x0c + [uidAsASCII length]);
+
+  prefix = @"040000008200e00074c5b7101a82e008";
+  uint8_t dataPrefix[] = { 0x76, 0x43, 0x61, 0x6c, 0x2d, 0x55, 0x69, 0x64, 0x01, 0x00, 0x00, 0x00 };
 
   binPrefix = [prefix convertHexStringToBytes];
-  [binPrefix getBytes: &newGlobalId.ByteArrayID];
-  [self _setInstanceDate: &newGlobalId
+  [binPrefix getBytes:&newGlobalId->ByteArrayID];
+  [self _setInstanceDate: newGlobalId
                 fromDate: [event recurrenceId]];
-  uidAsASCII = [uid dataUsingEncoding: NSASCIIStringEncoding];
   bytes = [uidAsASCII bytes];
 
   // 0x0c is the size of our dataPrefix
-  newGlobalId.Size = 0x0c + [uidAsASCII length];
-  newGlobalId.Data = malloc(newGlobalId.Size * sizeof(uint8_t));
-  memcpy(newGlobalId.Data, dataPrefix, 0x0c);
-  memcpy(newGlobalId.Data + 0x0c, bytes, newGlobalId.Size - 0x0c);
-
-  globalObjectId = [[NSData alloc] initWithBytes: &newGlobalId  length: 40 + newGlobalId.Size*sizeof(uint8_t)];
-  free(newGlobalId.Data);
+  newGlobalId->Size = 0x0c + [uidAsASCII length];
+  memcpy(newGlobalId->Data, dataPrefix, 0x0c);
+  memcpy(newGlobalId->Data + 0x0c, bytes, newGlobalId->Size - 0x0c);
+
+  globalObjectId =
+      [[NSData alloc] initWithBytes:newGlobalId
+                             length:40 + newGlobalId->Size * sizeof(uint8_t)];
+  free(newGlobalId);
   
   return [globalObjectId autorelease];
 }
patch.diff (2,479 bytes)   
zj5121

zj5121

2015-06-11 19:15

reporter   ~0008626

sorry the patch is wrong. don't use i please, I will attach right one later.
thanks.

ludovic

ludovic

2015-06-19 18:09

administrator   ~0008655

Any updates on this? Thanks!

zj5121

zj5121

2015-06-30 17:28

reporter  

globalid.diff (3,552 bytes)   
diff --git a/ActiveSync/SOGoMailObject+ActiveSync.m b/ActiveSync/SOGoMailObject+ActiveSync.m
index 164abb6..3e6de01 100644
--- a/ActiveSync/SOGoMailObject+ActiveSync.m
+++ b/ActiveSync/SOGoMailObject+ActiveSync.m
@@ -77,6 +77,41 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 #import <Appointments/SOGoAptMailNotification.h>
 
+unsigned char strToChar(char a, char b) {
+    char encoder[3] = {'\0','\0','\0'};
+    encoder[0] = a;
+    encoder[1] = b;
+    return (char) strtol(encoder,NULL,16);
+}
+
+@interface NSString (NSStringExtensions)
+- (NSData *) decodeFromHexidecimal;
+@end
+
+@implementation NSString (NSStringExtensions)
+
+- (NSData *) decodeFromHexidecimal;
+{
+    const char * bytes = [self cStringUsingEncoding: NSUTF8StringEncoding];
+    NSUInteger length = strlen(bytes);
+    unsigned char * r = (unsigned char *) malloc(length / 2 + 1);
+    unsigned char * index = r;
+
+    while ((*bytes) && (*(bytes +1))) {
+        *index = strToChar(*bytes, *(bytes +1));
+        index++;
+        bytes+=2;
+    }
+    *index = '\0';
+
+    NSData * result = [NSData dataWithBytes: r length: length / 2];
+    free(r);
+
+    return result;
+}
+
+@end
+
 typedef struct {
   uint32_t dwLowDateTime;
   uint32_t dwHighDateTime;
@@ -91,7 +126,7 @@ struct GlobalObjectId {
   FILETIME                  CreationTime;
   uint8_t                   X[8];
   uint32_t                  Size;
-  uint8_t*                  Data;
+  uint8_t Data[0];
 };
 
 @implementation SOGoMailObject (ActiveSync)
@@ -118,39 +153,40 @@ struct GlobalObjectId {
 //
 // The GlobalObjId is documented here: http://msdn.microsoft.com/en-us/library/ee160198(v=EXCHG.80).aspx
 //
+
 - (NSData *) _computeGlobalObjectIdFromEvent: (iCalEvent *) event
 {
   NSData *binPrefix, *globalObjectId, *uidAsASCII;
   NSString *prefix, *uid;
-
-  struct GlobalObjectId newGlobalId;
+  struct GlobalObjectId *newGlobalId;
   const char *bytes;
-  
+
+  uid = [event uid];
+  uidAsASCII = [uid decodeFromHexidecimal];
+  newGlobalId = (struct GlobalObjectId*)calloc(sizeof(uint8_t), sizeof(struct GlobalObjectId) + 0x0c + [uidAsASCII length]);
+
   prefix = @"040000008200e00074c5b7101a82e008";
 
   // dataPrefix is "vCal-Uid %x01 %x00 %x00 %x00"
   uint8_t dataPrefix[] = { 0x76, 0x43, 0x61, 0x6c, 0x2d, 0x55, 0x69, 0x64, 0x01, 0x00, 0x00, 0x00 };
-  uid = [event uid];
 
   binPrefix = [prefix convertHexStringToBytes];
-  [binPrefix getBytes: &newGlobalId.ByteArrayID];
-  [self _setInstanceDate: &newGlobalId
+  [binPrefix getBytes: &newGlobalId->ByteArrayID];
+  [self _setInstanceDate: newGlobalId
                 fromDate: [event recurrenceId]];
-  uidAsASCII = [uid dataUsingEncoding: NSASCIIStringEncoding];
   bytes = [uidAsASCII bytes];
 
   // 0x0c is the size of our dataPrefix
-  newGlobalId.Size = 0x0c + [uidAsASCII length];
-  newGlobalId.Data = malloc(newGlobalId.Size * sizeof(uint8_t));
-  memcpy(newGlobalId.Data, dataPrefix, 0x0c);
-  memcpy(newGlobalId.Data + 0x0c, bytes, newGlobalId.Size - 0x0c);
+  newGlobalId->Size = 0x0c + [uidAsASCII length];
+  memcpy(newGlobalId->Data, dataPrefix, 0x0c);
+  memcpy(newGlobalId->Data + 0x0c, bytes, newGlobalId->Size - 0x0c);
 
-  globalObjectId = [[NSData alloc] initWithBytes: &newGlobalId  length: 40 + newGlobalId.Size*sizeof(uint8_t)];
-  free(newGlobalId.Data);
-  
+  globalObjectId = [[NSData alloc] initWithBytes: newGlobalId  length: 40 + newGlobalId->Size*sizeof(uint8_t)];
+  free(newGlobalId);
   return [globalObjectId autorelease];
 }
 
+
 //
 // For debugging purposes...
 //
globalid.diff (3,552 bytes)   
zj5121

zj5121

2015-06-30 17:29

reporter   ~0008681

patch attached, please have a look.
filename: globalid.diff

ludovic

ludovic

2015-07-22 14:01

administrator   ~0008746

https://github.com/inverse-inc/sogo/commit/d2640e1501708fcf38f8887e26b14378aae73690

Issue History

Date Modified Username Field Change
2015-06-05 20:06 zj5121 New Issue
2015-06-09 14:19 ludovic Note Added: 0008606
2015-06-10 19:52 zj5121 File Added: patch.diff
2015-06-11 19:15 zj5121 Note Added: 0008626
2015-06-19 18:09 ludovic Note Added: 0008655
2015-06-30 17:28 zj5121 File Added: globalid.diff
2015-06-30 17:29 zj5121 Note Added: 0008681
2015-07-22 14:01 ludovic Note Added: 0008746
2015-07-22 14:01 ludovic Status new => resolved
2015-07-22 14:01 ludovic Fixed in Version => 2.3.1
2015-07-22 14:01 ludovic Resolution open => fixed
2015-07-22 14:01 ludovic Assigned To => ludovic