Revision 30:db87179335d5, committed 2016-07-13

Comitter:

LancasterUniversity

Date:

Wed Jul 13 12:18:08 2016 +0100

Parent:

29:62f8b007debf

Child:

31:87789e55bac7

Commit message:

Synchronized with git rev 3b435c0d
Author: James Devine
microbit-dal: BUGFIX in MicroBitStorage

There was an off by one error when storing the key of the key value
pair, where the null terminator was dropped. This would mean that if
the returned key of the KeyValuePair were used, it would cause a number
of issues.

Another issue raised was the copying a random 48 bytes from memory
regardless of the position of memory in the stack. If the memory was
smaller than 48 bytes, and existed at the top of the stack, this could
have dire consequences. As a result, MicroBitStorage now accepts a size
parameter which informs the number of bytes to be copied into flash.

#130

Changed in this revision

--- a/inc/drivers/MicroBitStorage.h	Wed Jul 13 12:18:07 2016 +0100
+++ b/inc/drivers/MicroBitStorage.h	Wed Jul 13 12:18:08 2016 +0100
@@ -162,9 +162,13 @@
       *
       * @param data a pointer to the beginning of the data to be persisted.
       *
-      * @return MICROBIT_OK on success, or MICROBIT_NO_RESOURCES if the storage page is full
+      * @param dataSize the size of the data to be persisted
+      *
+      * @return MICROBIT_OK on success, MICROBIT_INVALID_PARAMETER if the key or size is too large,
+      *         MICROBIT_NO_RESOURCES if the storage page is full
       */
-    int put(const char* key, uint8_t* data);
+    int put(const char* key, uint8_t* data, int dataSize);
+
 
     /**
       * Places a given key, and it's corresponding value into flash at the earliest
@@ -174,9 +178,12 @@
       *
       * @param data a pointer to the beginning of the data to be persisted.
       *
-      * @return MICROBIT_OK on success, or MICROBIT_NO_RESOURCES if the storage page is full
+      * @param dataSize the size of the data to be persisted
+      *
+      * @return MICROBIT_OK on success, MICROBIT_INVALID_PARAMETER if the key or size is too large,
+      *         MICROBIT_NO_RESOURCES if the storage page is full
       */
-    int put(ManagedString key, uint8_t* data);
+    int put(ManagedString key, uint8_t* data, int dataSize);
 
     /**
       * Retreives a KeyValuePair identified by a given key.

--- a/source/bluetooth/MicroBitBLEManager.cpp	Wed Jul 13 12:18:07 2016 +0100
+++ b/source/bluetooth/MicroBitBLEManager.cpp	Wed Jul 13 12:18:08 2016 +0100
@@ -99,7 +99,7 @@
         if(memcmp(attribStore.sys_attrs[deviceID].sys_attr, attrib.sys_attr, len) != 0)
         {
             attribStore.sys_attrs[deviceID] = attrib;
-            manager->storage->put(key, (uint8_t *)&attribStore);
+            manager->storage->put(key, (uint8_t *)&attribStore, sizeof(attribStore));
         }
     }
 }

--- a/source/drivers/MicroBitCompass.cpp	Wed Jul 13 12:18:07 2016 +0100
+++ b/source/drivers/MicroBitCompass.cpp	Wed Jul 13 12:18:08 2016 +0100
@@ -703,7 +703,7 @@
 void MicroBitCompass::setCalibration(CompassSample calibration)
 {
     if(this->storage != NULL)
-        this->storage->put(ManagedString("compassCal"), (uint8_t *)&calibration);
+        this->storage->put(ManagedString("compassCal"), (uint8_t *)&calibration, sizeof(CompassSample));
 
     average = calibration;
     status |= MICROBIT_COMPASS_STATUS_CALIBRATED;
@@ -775,4 +775,4 @@
     {3200000,    0xd0},        // 0.31 hz
     {6400000,    0xf0},        // 0.16 hz
     {12800000,   0xf8}         // 0.08 hz
-};
+};
\ No newline at end of file

--- a/source/drivers/MicroBitStorage.cpp	Wed Jul 13 12:18:07 2016 +0100
+++ b/source/drivers/MicroBitStorage.cpp	Wed Jul 13 12:18:08 2016 +0100
@@ -209,14 +209,22 @@
   *
   * @param data a pointer to the beginning of the data to be persisted.
   *
-  * @return MICROBIT_OK on success, or MICROBIT_NO_RESOURCES if the storage page is full
+  * @param dataSize the size of the data to be persisted
+  *
+  * @return MICROBIT_OK on success, MICROBIT_INVALID_PARAMETER if the key or size is too large,
+  *         MICROBIT_NO_RESOURCES if the storage page is full
   */
-int MicroBitStorage::put(const char *key, uint8_t *data)
+int MicroBitStorage::put(const char *key, uint8_t *data, int dataSize)
 {
     KeyValuePair pair = KeyValuePair();
 
-    memcpy(pair.key, key, min(sizeof(pair.key), strlen(key)));
-    memcpy(pair.value, data, sizeof(pair.value));
+    int keySize = strlen(key) + 1;
+
+    if(keySize > (int)sizeof(pair.key) || dataSize > (int)sizeof(pair.value) || dataSize < 0)
+        return MICROBIT_INVALID_PARAMETER;
+
+    memcpy(pair.key, key, keySize);
+    memcpy(pair.value, data, dataSize);
 
     //calculate our various offsets.
     uint32_t pg_size = NRF_FICR->CODEPAGESIZE;
@@ -290,11 +298,14 @@
   *
   * @param data a pointer to the beginning of the data to be persisted.
   *
-  * @return MICROBIT_OK on success, or MICROBIT_NO_RESOURCES if the storage page is full
+  * @param dataSize the size of the data to be persisted
+  *
+  * @return MICROBIT_OK on success, MICROBIT_INVALID_PARAMETER if the key or size is too large,
+  *         MICROBIT_NO_RESOURCES if the storage page is full
   */
-int MicroBitStorage::put(ManagedString key, uint8_t* data)
+int MicroBitStorage::put(ManagedString key, uint8_t* data, int dataSize)
 {
-    return put((char *)key.toCharArray(), data);
+    return put((char *)key.toCharArray(), data, dataSize);
 }
 
 /**
@@ -482,4 +493,4 @@
     }
 
     return store.size;
-}
+}
\ No newline at end of file

--- a/source/drivers/MicroBitThermometer.cpp	Wed Jul 13 12:18:07 2016 +0100
+++ b/source/drivers/MicroBitThermometer.cpp	Wed Jul 13 12:18:08 2016 +0100
@@ -245,7 +245,7 @@
 int MicroBitThermometer::setOffset(int offset)
 {
     if(this->storage != NULL)
-        this->storage->put(ManagedString("tempCal"), (uint8_t *)&offset);
+        this->storage->put(ManagedString("tempCal"), (uint8_t *)&offset, sizeof(int));
 
     this->offset = offset;
 
@@ -275,4 +275,4 @@
 {
     updateSample();
     return setOffset(temperature - calibrationTemp);
-}
+}
\ No newline at end of file