Merge pull request #14625 from paul-szczepanek-arm/fix-read-auth

BLE: Fix overwriting attribute data from read auth callback

Author: 0xc0170

connectivity/FEATURE_BLE/include/ble/gatt/GattCharacteristic.h

@@ -1672,19 +1672,30 @@ class GattCharacteristic {
      *
      * @attention This function is not meant to be called by user code.
      *
-     * @param[in] params Context of the read-auth request; it contains an
+     * @param[in,out] params Context of the read-auth request; it contains an
      * out-parameter used as a reply and the handler can fill it with outgoing
-     * data.
+     * data. The params->data provides a pointer to the data and params->len
+     * provides the length of this data. params->len is also used to pass the
+     * maximum size of data that the params->data can contain. If you set the
+     * params->len to a value larger than the passed in value the read operation
+     * will fail.
      *
      * @return A GattAuthCallbackReply_t value indicating whether authorization
      * is granted.
      *
+     * @note If the read is approved, the event handler can specify an outgoing
+     * value directly with the help of the fields params->data and params->len.
+     *
      * @note If the read request is approved and params->data remains nullptr, then
      * the current characteristic value is used in the read response payload.
      *
-     * @note If the read is approved, the event handler can specify an outgoing
-     * value directly with the help of the fields
-     * GattReadAuthCallbackParams::data and GattReadAuthCallbackParams::len.
+     * @note The params->len parameter initially contains the maximum length of
+     * data that can be returned. Set it to the length of your data but it must
+     * not be larger than the original value.
+     *
+     * @note You must also take into account the offset provided in params->offset.
+     * The params->len you provide must be larger then the offset as the read operation
+     * will attempt to read at that offset.
      */
     GattAuthCallbackReply_t authorizeRead(GattReadAuthCallbackParams *params)
     {

connectivity/FEATURE_BLE/source/cordio/source/GattServerImpl.cpp

@@ -1124,13 +1124,15 @@ uint8_t GattServer::atts_read_cb(
     attsAttr_t *pAttr
 )
 {
+    uint8_t err = ATT_SUCCESS;
+
     char_auth_callback *auth_cb = getInstance().get_auth_callback(handle);
     if (auth_cb && auth_cb->read_cb) {
         GattReadAuthCallbackParams read_auth_params = {
             connId,
             handle,
             offset,
-            /* len */ 0,
+            /* len */ pAttr->maxLen,
             /* data */ nullptr,
             AUTH_CALLBACK_REPLY_SUCCESS
         };
@@ -1146,8 +1148,23 @@ uint8_t GattServer::atts_read_cb(
             return read_auth_params.authorizationReply & 0xFF;
         }
 
-        pAttr->pValue = read_auth_params.data;
-        *pAttr->pLen = read_auth_params.len;
+        /* if new data provided copy into the attribute value buffer */
+        if (read_auth_params.data) {
+            if (read_auth_params.len > pAttr->maxLen) {
+                tr_error("Read authorisation callback set length larger than maximum attribute length, "
+                         "cannot copy data");
+                err = ATT_ERR_UNLIKELY;
+            } else {
+                memcpy(pAttr->pValue, read_auth_params.data, read_auth_params.len);
+                *pAttr->pLen = read_auth_params.len;
+
+                if (read_auth_params.len < offset) {
+                    tr_warning("Read authorisation callback shortened data beyond current offset, "
+                               "current read will fail");
+                    err = ATT_ERR_OFFSET;
+                }
+            }
+        }
     }
 
     tr_debug("Read attribute %d on connection %d - value=%s",
@@ -1161,11 +1178,11 @@ uint8_t GattServer::atts_read_cb(
         offset,
         *pAttr->pLen,
         pAttr->pValue,
-        /* status */ BLE_ERROR_NONE,
+        /* status */ (err == ATT_SUCCESS) ? BLE_ERROR_NONE : BLE_ERROR_PARAM_OUT_OF_RANGE
     };
     getInstance().handleDataReadEvent(&read_params);
 
-    return ATT_SUCCESS;
+    return err;
 }
 
 uint8_t GattServer::atts_write_cb(