AWSDynamoDBObjectMapper scan function causes a crash

[sha8wn]

Hello AWS team,

Describe the bug I create an AWSDynamoDBObjectMapper subclass to help me pull some data from our AWSDynamoDB instance.

Everything seemed to be working fine for the last couple of weeks until today I found my app was crashing with the following:

The two issues I notice are:

• It seems the AWSDynamoDBObjectMapper tries to retrieve all the properties before checking if the AWSDynamoDBObjectMapper needs them as the property causing the crash addressLineOne is not part of my AWSDynamoDBObjectMapper subclass
• It seems that nil properties within dictionaries / objects are not handled

To Reproduce Steps to reproduce the behavior:

1. Create an AWSDynamoDBObjectMapper and try to do call a basic scan function to pull database from a AWSDynamoDB table
2. Return an object that AWSDynamoDBObjectMapper sees as a Map which is a probably dictionary
3. Have a nil value for one of the properties within the map
4. The app should crash when parsing the output

Expected Behavior AWSDynamoDBObjectMapper handles nil values in maps

Observed Behavior

• I believe the the cause for the crash is is the function - (id)aws_getAttributeValue in AWSDynamoDBObjectMapper
• The issue seems to result when handling the self.M case which recursively calls aws_getAttributeValue
• If one of the values of the map is nil, aws_getAttributeValue returns nil and this cannot be set as a value of a dictionary which causes the crash

Potential Fix I have replaced this block:

else if (self.M) {
        NSMutableDictionary *map = [NSMutableDictionary dictionaryWithCapacity:self.M.count];
        for (NSString *entryAttributeKey in self.M) {
            id entryAttributeValue = self.M[entryAttributeKey];
            [map setObject:[entryAttributeValue aws_getAttributeValue]
                    forKey:entryAttributeKey];
        }
        return map;
}

with

else if (self.M) {
        NSMutableDictionary *map = [NSMutableDictionary dictionaryWithCapacity:self.M.count];
        for (NSString *entryAttributeKey in self.M) {
            id entryAttributeValue = self.M[entryAttributeKey];
          
          if (![entryAttributeValue aws_getAttributeValue]) {
            [map setObject:[NSNull null]
                      forKey:entryAttributeKey];
          }
          else {
            [map setObject:[entryAttributeValue aws_getAttributeValue]
                      forKey:entryAttributeKey];
          }
        }
        return map;
 }

which seems to resolve the crash for now.

Environment(please complete the following information):

• SDK Version: 2.23.2
• Dependency Manager: CocoaPods
• Swift Version : 5.0
• Xcode Version: 12.4

Device Information (please complete the following information):

• Device: iPad Pro
• iOS Version: 14.4

[618coffee]

Hi, @sha8wn Thanks for reaching out.

There are few questions I have

1. What dependencies are you using? just AWSDynamoDB?
2. Are you using AWSAppSync to first send some data to DynamoDB first?
3. How do you configure DynomoDBObjectMapper?
4. What does the DynamoDB part looks like in you awsconfiguration.json?
5. If the answer to Q2 is yes, could you share the schema that you are using?

[sha8wn]

Hello @ruiguoamz ,

1. What dependencies are you using? just AWSDynamoDB? AWSDynamoDB AWSCore

2. Are you using AWSAppSync to first send some data to DynamoDB first? No, I just use scan to retrieve data

3. How do you configure DynomoDBObjectMapper?

@objcMembers
class AWSTransaction: AWSDynamoDBObjectModel, AWSDynamoDBModeling {
  
  var pk = ""
  var category = ""
  var top_level_category = ""
  var date = ""
  var amount = NSNumber(0.0)
  var currency_code = ""
  var original_description = ""
  
  static func hashKeyAttribute() -> String {
    return AWSConstants.DYNAMODB_DB_TRANSACTIONS_TABLE_PK
  }
  
  static func dynamoDBTableName() -> String {
    return AWSConstants.DYNAMODB_DB_TRANSACTIONS_TABLE
  }
  
  override init!() { super.init() }
  
  override init(dictionary dictionaryValue: [AnyHashable : Any]!, error: ()) throws {
    try super.init(dictionary: dictionaryValue, error: error)
  }
  
  required init!(coder: NSCoder!) {
    super.init(coder: coder)
  }
}

4. What does the DynamoDB part looks like in you awsconfiguration.json? I do not configure using a JSON file but I configure directly via code using Cognito

func authenticateWithCognito(inEnvironment environment: AWSEnvironment) {
    var identityPoolId = AWSConstants.COGNITO_IDENTITY_POOL_ID_DEVELOPMENT
    
    let credentialsProvider = AWSCognitoCredentialsProvider(regionType: AWSConstants.COGNITO_REGIONTYPE,
                                                            identityPoolId: identityPoolId)
    
    let serviceConfiguration = AWSServiceConfiguration(region: AWSConstants.COGNITO_REGIONTYPE,
                                                       credentialsProvider: credentialsProvider)

    AWSServiceManager.default().defaultServiceConfiguration = serviceConfiguration
}

func getTransactionData(fromEnvironment environment: AWSEnvironment, completion: (([AWSTransaction]?, NSError?) -> Void)?) {
    authenticateWithCognito(inEnvironment: environment)
    guard let scanInput = AWSDynamoDBScanInput() else {
      return
      // handle errors
    }
    
    scanInput.tableName = AWSConstants.DYNAMODB_DB_TRANSACTIONS_TABLE
    
    let mapper = AWSDynamoDBObjectMapper.default()
    let exp = AWSDynamoDBScanExpression()
    exp.limit = 100
    mapper.scan(AWSTransaction.self, expression: exp) { (scanOutput, error) in    //Crash occurs here
      print(scanOutput) 
    }
}

Important to note, this same set up did not crash before as my backend team never sent any Map/Dictionary data with null values and as you can see from my original question, I am not even specifying the property causing the crash in my model class.

5. If the answer to Q2 is yes, could you share the schema that you are using? Answer to Q2 is no, I could check with my backend team however I suspect this could take a while for me to get this. Hoping the data I provided is enough for now.

Thanks

[rromanchuk]

This is my favorite product in the SDK, but when it comes toAWSDynamoDBObjectModel, AWSDynamoDBModeling bridging between objective c/swift is an absolute minefield of danger. Especially true if you have other sources writing to the document.

Maps/Sets are by far the worst to defend against.

Oddly enough, i am also getting an uptick of crashes here too. It's a nightmare to debug if you can't immediately reproduce with debugger attached because of the stack frame is worthless and you basically have to start fishing around payloads just to find the key that is throwing

[rromanchuk]

It seems the AWSDynamoDBObjectMapper tries to retrieve all the properties before checking if the AWSDynamoDBObjectMapper needs them as the property causing the crash addressLineOne is not part of my AWSDynamoDBObjectMapper subclass

@sha8wn this just saved me hours of debugging. This is exactly why i'm seeing a key that does not even exist on the model show up in some logs.

This certainly feels like a regression, or perhaps i've been miraculously lucky before?

[palpatim]

This certainly feels like a regression, or perhaps i've been miraculously lucky before?

We haven't touched this code in quite a while other than standard version bumps.

@sha8wn

Return an object that AWSDynamoDBObjectMapper sees as a Map which is a probably dictionary Have a nil value for one of the properties within the map

Do you have an example of such an object? As noted above, we rarely touch this code, so anything you can do to jump-start the investigation would be appreciated.

[sha8wn]

Hello @palpatim - I believe my initial post had the way to reproduce this issue and also how I solved it. This is the issue at a high level is handling nil values for objects perceived as Maps in my opinion.

A more deeper dive as follows:

• In an AWSDynamoDB table, there is a possibility to create a sophisticated object as a property. I believe this is something like a class / dictionary type object. I am not a backend expert and have no idea of the exact DB schema, so I cannot confirm the exact type, however such objects I guess are referred to as Maps as per what I saw in your library.

• There key thing I noticed here is that there is a a chance that the above object is nullable and such Map objects are parsed as NSMutableDictionaries in the AWSDynamoDBObjectMapper

• The data is parsed in the - (id)aws_getAttributeValue in the AWSDynamoDBObjectMapper tries to solve null cases by recursively calling the function to get null values for different types - the issue is you cannot set a nil value for in an Objective C dictionary. This is what is happing in your code here.

• I just made a small change to this recursive call and notice that if the object is of type Map and I use the safe [NSNull null] object to handle the nil case for Maps / dictionaries which resolves the crash which I show here

I hope this helps.

Thanks

[royjit]

Does your schema has an attribute of type "NULL", looks like this is not supported in AWSDynamoDBObjectMapper. It could be helpful for us if you can provide a sample dynamodb entry that would case the crash.

[sha8wn]

Does your schema has an attribute of type "NULL", looks like this is not supported in AWSDynamoDBObjectMapper. It could be helpful for us if you can provide a sample dynamodb entry that would case the crash.

Unfortunately I am no longer working on that project and no longer have access to the database but I can say with quite a high level of certainty, yes, it is because the schema has an attribute which has a type NULL.

Handling a NULL object works well for most cases, there is a type in AWSDynamoDB which gets converted into a Map when being parsed in the AWSDynamoDBObjectMapper

If the value of a type Map is NULL, the crash seems to occur. I believe since you are with AWS, you should be able to reproduce this at your end given the above explanation.

Unfortunately I no longer work with the project anymore so I cannot request the schema.

[royjit]

AWSDynamoDBObjectMapper in the aws-sdk-ios does not support Null as per these comments - https://github.com/aws-amplify/aws-sdk-ios/blob/ca7d8a242a5c0e3242d11fdb3783685666408bb3/AWSDynamoDB/AWSDynamoDBObjectMapper.m#L140 , marking this as feature request.