Fix string out of bounds for asset file opening. #4673
Conversation
When opening the assets folder as a file, an out-of-bounds string pointer is passed to AAssetManager_open. b/388599092
| AAsset* OpenAndroidAsset(const char* path) { | ||
| if (!IsAndroidAssetPath(path) || g_asset_manager == NULL) { | ||
| SB_LOG(WARNING) << "Unable to open from Android Asset Manager: " << path; | ||
| if (!IsAndroidAssetFile(path) || g_asset_manager == NULL) { | ||
| errno = ENOENT; | ||
| return NULL; | ||
| } | ||
| const char* asset_path = path + strlen(g_app_assets_dir) + 1; |
There was a problem hiding this comment.
should we check asset_path has content here?
IsAndroidAssetFile() above only check path has the asset directory + '/', maybe we should check it has content after '/' there, and it does not end with '/'?
There was a problem hiding this comment.
Is the memory pointed by the path pointer corrupted? What is causing that?
There was a problem hiding this comment.
Before this PR, the "+1" on line 124 points out of bounds when the asset directory is passed into OpenAndroidAsset.
There was a problem hiding this comment.
Re colin: I don't want to change the logic in this PR, only fix the out-of-bounds pointer resulting in a crash. If the asset directory + '/' is passed, the function behaves the same before and after this PR.
There was a problem hiding this comment.
AAssetManager_open can handle empty asset_path? Sounds good to me then.
There was a problem hiding this comment.
My point is that even if it can't handle it, that is a separate bug that exists before and after this PR, and it out of scope for this PR.
jellefoks
requested review from
zhongqiliang and
y4vor
and removed request for
y4vor
|
Gentle ping? |
When opening the assets folder as a file, an out-of-bounds string pointer is passed to AAssetManager_open.
Also remove some logspam.
b/388599092