From e2d1e3d0436aec645739c65e6d3131dd814f40a1 Mon Sep 17 00:00:00 2001 From: Igor Murashkin Date: Tue, 30 Apr 2013 18:18:06 -0700 Subject: camera: Use new camera_metadata structure validation functions * Reject unvalidated metadata across binder boundaries * Sanity check in-process CameraMetadata when mutating data Bug: 8713951 Change-Id: I121d8e15f8fdc9cdbbaf27dfd947813e11831e1c --- camera/CameraMetadata.cpp | 15 +++++++++++ camera/IProCameraUser.cpp | 63 ++++++++++++++++++++++++++++++++++++----------- 2 files changed, 63 insertions(+), 15 deletions(-) (limited to 'camera') diff --git a/camera/CameraMetadata.cpp b/camera/CameraMetadata.cpp index 6c3e233..a8f9eff 100644 --- a/camera/CameraMetadata.cpp +++ b/camera/CameraMetadata.cpp @@ -14,6 +14,8 @@ * limitations under the License. */ +// #define LOG_NDEBUG 0 + #define LOG_TAG "Camera2-Metadata" #include #include @@ -112,6 +114,10 @@ void CameraMetadata::acquire(camera_metadata_t *buffer) { } clear(); mBuffer = buffer; + + ALOGE_IF(validate_camera_metadata_structure(mBuffer, /*size*/NULL) != OK, + "%s: Failed to validate metadata structure %p", + __FUNCTION__, buffer); } void CameraMetadata::acquire(CameraMetadata &other) { @@ -289,6 +295,15 @@ status_t CameraMetadata::updateImpl(uint32_t tag, const void *data, __FUNCTION__, get_camera_metadata_section_name(tag), get_camera_metadata_tag_name(tag), tag, strerror(-res), res); } + + IF_ALOGV() { + ALOGE_IF(validate_camera_metadata_structure(mBuffer, /*size*/NULL) != + OK, + + "%s: Failed to validate metadata structure after update %p", + __FUNCTION__, mBuffer); + } + return res; } diff --git a/camera/IProCameraUser.cpp b/camera/IProCameraUser.cpp index 0c94bd4..4c4dec3 100644 --- a/camera/IProCameraUser.cpp +++ b/camera/IProCameraUser.cpp @@ -50,17 +50,30 @@ enum { * Caller becomes the owner of the new metadata * 'const Parcel' doesnt prevent us from calling the read functions. * which is interesting since it changes the internal state + * + * NULL can be returned when no metadata was sent, OR if there was an issue + * unpacking the serialized data (i.e. bad parcel or invalid structure). */ void readMetadata(const Parcel& data, camera_metadata_t** out) { - camera_metadata_t* metadata; + + status_t err = OK; + + camera_metadata_t* metadata = NULL; + + if (out) { + *out = NULL; + } // arg0 = metadataSize (int32) - size_t metadataSize = static_cast(data.readInt32()); + int32_t metadataSizeTmp = -1; + if ((err = data.readInt32(&metadataSizeTmp)) != OK) { + ALOGE("%s: Failed to read metadata size (error %d %s)", + __FUNCTION__, err, strerror(-err)); + return; + } + const size_t metadataSize = static_cast(metadataSizeTmp); if (metadataSize == 0) { - if (out) { - *out = NULL; - } return; } @@ -70,21 +83,23 @@ void readMetadata(const Parcel& data, camera_metadata_t** out) { ReadableBlob blob; // arg1 = metadata (blob) - { - data.readBlob(metadataSize, &blob); + do { + if ((err = data.readBlob(metadataSize, &blob)) != OK) { + ALOGE("%s: Failed to read metadata blob (sized %d). Possible " + " serialization bug. Error %d %s", + __FUNCTION__, metadataSize, err, strerror(-err)); + break; + } const camera_metadata_t* tmp = reinterpret_cast(blob.data()); - size_t entry_capacity = get_camera_metadata_entry_capacity(tmp); - size_t data_capacity = get_camera_metadata_data_capacity(tmp); - metadata = allocate_camera_metadata(entry_capacity, data_capacity); - copy_camera_metadata(metadata, metadataSize, tmp); - } + metadata = allocate_copy_camera_metadata_checked(tmp, metadataSize); + } while(0); blob.release(); if (out) { *out = metadata; - } else { + } else if (metadata != NULL) { free_camera_metadata(metadata); } } @@ -95,14 +110,13 @@ void readMetadata(const Parcel& data, camera_metadata_t** out) { */ void writeMetadata(Parcel& data, camera_metadata_t* metadata) { // arg0 = metadataSize (int32) - size_t metadataSize; if (metadata == NULL) { data.writeInt32(0); return; } - metadataSize = get_camera_metadata_compact_size(metadata); + const size_t metadataSize = get_camera_metadata_compact_size(metadata); data.writeInt32(static_cast(metadataSize)); // arg1 = metadata (blob) @@ -110,6 +124,25 @@ void writeMetadata(Parcel& data, camera_metadata_t* metadata) { { data.writeBlob(metadataSize, &blob); copy_camera_metadata(blob.data(), metadataSize, metadata); + + IF_ALOGV() { + if (validate_camera_metadata_structure( + (const camera_metadata_t*)blob.data(), + &metadataSize) != OK) { + ALOGV("%s: Failed to validate metadata %p after writing blob", + __FUNCTION__, blob.data()); + } else { + ALOGV("%s: Metadata written to blob. Validation success", + __FUNCTION__); + } + } + + // Not too big of a problem since receiving side does hard validation + if (validate_camera_metadata_structure(metadata, &metadataSize) != OK) { + ALOGW("%s: Failed to validate metadata %p before writing blob", + __FUNCTION__, metadata); + } + } blob.release(); } -- cgit v1.1