Segmentation fault in Aws::S3::S3Client::GetObject #781
Comments
|
I can reproduce both crash and weird behavior when customer's S3 object has #include <aws/s3/S3Client.h>
#include <aws/s3/model/GetObjectRequest.h>
#include <aws/core/Aws.h>
int main(int argc, char *argv[])
{
Aws::SDKOptions options;
Aws::InitAPI(options);
Aws::Client::ClientConfiguration config;
config.region = /*cut*/;
auto client = Aws::MakeShared<Aws::S3::S3Client>("s3-get", config, Aws::Client::AWSAuthV4Signer::PayloadSigningPolicy::Never);
Aws::S3::Model::GetObjectRequest request;
request.SetBucket(/*cut*/);
request.SetKey(/*cut*/);
auto outcome = client->GetObject(request); // Crashes here on specific S3 objects
if (!outcome.IsSuccess()) {
std::cerr << "Error " << static_cast<int>(outcome.GetError().GetErrorType()) << ": " << outcome.GetError().GetMessage().c_str() << "\n";
return 1;
}
std::cout << outcome.GetResult().GetBody().rdbuf();
return 0;
}I've tested it against two files and get different failures. Note that I've tested it on macOS but it should be the same on any other platform. Note that I've verified that if I run the above program for a "normal" S3 object I get one's content printed. Case 1: Customer's S3 object is a particularly malformed XMLI got segmentation fault S3 file Segmentation fault call stack: Case 2: Customer's S3 object is valid XML with
|
|
Agreed re: cutting down the scope of where it's called. In our internal fix, which is different due to being on an older version, we only do this for calls that go via AttemptOneRequest and then only calls which are not GET, which is sufficient. It leaves open the possibility that future API calls will have the same behavior. It could be cut down even more and/or have a different entry point for where we check to see if it's a call which needs this additional check (besides AttemptOneRequest), but I strongly agree that limiting the scope just to calls where it's needed is a requirement. I share the same concern regarding performance. The memory use should also be much increased, especially for large objects. I've asked the developer who worked on that here to share the solution on this bug. |
|
Hi, we have updated TinyXml2 to the latest version and at the same time reverted the changes done in f5c0f79. We will find a more elegant solution for that problem. You should have no problem now on version 1.3.46 , please reopen it if you still have problem. |
|
Thank you, Andrew |
Uh oh!
There was an error while loading. Please reload this page.
We build SDK of version 47030e9 with GCC 4.9.4 and run it on Linux @ x86_64, although the issue should occur with any platform/OS/compiler.
Segfault Call stack:
The crash is happening in TinyXML2 because of the old bug in parsing malformed entities leethomason/tinyxml2#291
I'm quite sure that recent change f5c0f79 had exposed the TinyXML2 bug.
In short, this change tries to parse every S3 response body as XML document to extract an error from it. S3 response body is customer’s S3 object thus just some (random) bytes. Parsing every S3 object as XML effectively means performing a fuzzy testing of TinyXML2 which is apparently failed :-)
The possible solution for the crash would be to update TinyXML2 version used in the SDK. I've created a separate ticket for this (since it need to be done regardless of this issue): #777
In addition to the TinyXML2 fix I’m also concerned of the following:
CompleteMultipartUploadresponses since they’re only ones that can return 200 OK + error in the body? Extra option would be to parse only responses that are smaller than a certain size (128K, 1M?) because if it’s bigger it’s definitely not an error but customer’s S3 object.CompleteMultipartUploadresponses would fix the problem as well.For me it seems that scoping down response parsing to
CompleteMultipartUploadresponses is the only real solution for the issue.The text was updated successfully, but these errors were encountered: