We are looking for errors in the source code of the Amazon Web Services SDK for .NET

    Picture 1


    Greetings to all fans to criticize someone else's code. :) Today in our laboratory a new study material is the source code of the AWS SDK project for .NET. At one time we wrote an article about checking the AWS SDK for C ++. Then there was nothing particularly interesting. Let's see how we enjoy the .NET version of the AWS SDK. A good opportunity to once again demonstrate the capabilities of the PVS-Studio analyzer, as well as to make the world a little bit more perfect.

    The Amazon Web Services (AWS) SDK for .NET is a developer toolkit for building .NET-based applications in the AWS infrastructure and making it much easier to write code. The SDK includes .NET API sets for various AWS services, such as Amazon S3, Amazon EC2, DynamoDB, and others. The source code for the SDK is hosted on GitHub .

    As I said, we wrote an article in due time.about checking the AWS SDK for C ++. The article turned out to be small - only a couple of errors were found for 512 thousand lines of code. This time we are dealing with a much larger amount of code, which includes about 34 thousand cs-files, and the total number of lines of code (excluding empty ones) is an impressive 5 million. A small part of the code (200 thousand lines in 664 cs-files) falls on tests, I have not considered them.

    If the quality of the .NET SDK version is approximately the same as that of C ++ (two errors per 512 KLOC), then we should get about 10 times more errors. This, of course, is a very inaccurate method of calculation, which does not take into account language features and many other factors, but I do not think that the reader now wants to delve into boring arguments. Instead, I suggest going straight to the results.

    The check was performed using PVS-Studio version 6.27. Incredibly, the analyzer was able to detect about 40 errors in the AWS SDK code for .NET, which would be worth telling. This indicates not only the high quality of the SDK code (approximately 4 errors per 512 KLOC), but also the comparable quality of the C # performance of the PVS-Studio analyzer compared to C ++. Excellent result!

    The authors of the AWS SDK for .NET are great fellows. From project to project, they demonstrate amazing code quality. This can serve as a good example for other teams. But, of course, I would not be a developer of a static analyzer if I had not inserted my 5 kopecks. :) We are already interacting with the Lumberyard team from Amazon on the use of PVS-Studio. But since this is a very large company with a lot of divisions around the world, it is very likely that the AWS SDK team for .NET never heard of PVS-Studio at all. In any case, in the SDK code I did not find any signs of using our analyzer, although this does not mean anything. However, the command, at a minimum, uses the analyzer built into Visual Studio. This is good, but the code checks can be strengthened :).

    As a result, I still managed to find a few errors in the SDK code and, finally, it was time to share them.

    Logic Error

    PVS-Studio Warning: V3008 [CWE-563] The 'this.linker.s3.region' variable is assigned values ​​twice successively. Perhaps this is a mistake. Check lines: 116, 114. AWSSDK.DynamoDBv2.Net45 S3Link.cs 116

    publicstring Region 
    { 
      get 
      {
        ....
      } 
      set 
      {
        if (String.IsNullOrEmpty(value))
        {
          this.linker.s3.region = "us-east-1";
        }
        this.linker.s3.region = value; 
      } 
    }

    The analyzer warns about the reassignment of the value of the same variable. From the code it becomes clear that this is due to an error that violates the logic of operation: the value of the variable this.linker.s3.region will always be equal to value , regardless of the condition if (String.IsNullOrEmpty (value)) . In the body of the if block , return was omitted . The code needs to be corrected as follows:

    publicstring Region
    { 
      get 
      {
        ....
      } 
      set 
      {
        if (String.IsNullOrEmpty(value))
        {
          this.linker.s3.region = "us-east-1";
          return;  
        }
        this.linker.s3.region = value; 
      } 
    }

    Infinite recursion

    PVS-Studio warning : V3110 [CWE-674] Possible infinite recursion inside 'OnFailure' property. AWSSDK.ElasticMapReduce.Net45 ResizeJobFlowStep.cs 171

    OnFailure? onFailure = null;
    public OnFailure? OnFailure
    {
      get { returnthis.OnFailure; }  // <=set { this.onFailure = value; }
    }

    The classic example of a typo that causes an infinite recursion in the get access method of the OnFailure property . Instead of returning the value of the private onFailure field , access the OnFailure property . Corrected version:

    public OnFailure? OnFailure
    {
      get { returnthis.onFailure; }
      set { this.onFailure = value; }
    }

    You ask: “How did it work?” So far - no way. The property is not used anywhere, but this is a temporary phenomenon. At one point, someone will start using it and will definitely get an unexpected result. To prevent such typos, it is recommended not to use identifiers that differ only in the case of the first letter.

    One more note to this construction is the use of an identifier that completely coincides with the name of the OnFailure type . From the point of view of the compiler, this is quite acceptable, but makes it difficult for a person to read the code.

    Another similar error:

    PVS-Studio warning : V3110 [CWE-674] Possible infinite recursion inside 'SSES3' property. AWSSDK.S3.Net45 InventoryEncryption.cs 37

    private SSES3 sSES3;
    public SSES3 SSES3
    {
      get { returnthis.SSES3; }
      set { this.SSES3 = value; }
    }

    The situation is identical to that described above. Only here infinite recursion will occur when accessing the SSES3 property for both reading and writing. Corrected version:

    public SSES3 SSES3
    {
      get { returnthis.sSES3; }
      set { this.sSES3 = value; }
    }

    Attention Test The

    following is a task from a programmer who is passionate about using the Copy-Paste method. Look at how the code looks in the Visual Studio editor, and try to find the error.

    Picture 3


    PVS-Studio warning : V3029 The terms expressions of the "if" are expressed alongside each other are identical. Check lines: 91, 95. AWSSDK.AppSync.Net45 CreateApiKeyResponseUnmarshaller.cs 91

    I shortened the body of the UnmarshallException method , removing all unnecessary. Now it is clear that identical checks follow one after the other:

    public override AmazonServiceException UnmarshallException(....){
      ....
      if (errorResponse.Code != null &&
        errorResponse.Code.Equals("LimitExceededException"))
      {
        returnnew LimitExceededException(errorResponse.Message,
          innerException, errorResponse.Type, errorResponse.Code,
          errorResponse.RequestId, statusCode);
      }
      if (errorResponse.Code != null &&
        errorResponse.Code.Equals("LimitExceededException"))
      {
        returnnew LimitExceededException(errorResponse.Message,
          innerException, errorResponse.Type, errorResponse.Code,
          errorResponse.RequestId, statusCode);
      }
      ....
    }

    It may seem that the error is not gross - just an extra check. But often such a pattern may indicate more serious problems in the code when some necessary verification is not performed.

    There are some more similar errors in the code.

    PVS-Studio warnings:

    • V3029 The conditional expressions of the statement "if" statements followed alongside each other are identical. Check lines: 75, 79. AWSSDK.CloudDirectory.Net45 CreateSchemaResponseUnmarshaller.cs 75
    • V3029 The conditional expressions of the statement "if" statements followed alongside each other are identical. Check lines: 105, 109. AWSSDK.CloudDirectory.Net45 GetSchemaAsJsonResponseUnmarshaller.cs 105
    • V3029 The conditional expressions of the 'if' statements situated alongside each other are identical. Check lines: 201, 205. AWSSDK.CodeCommit.Net45 PostCommentForPullRequestResponseUnmarshaller.cs 201
    • V3029 The conditional expressions of the 'if' statements situated alongside each other are identical. Check lines: 101, 105. AWSSDK.CognitoIdentityProvider.Net45 VerifySoftwareTokenResponseUnmarshaller.cs 101
    • V3029 The conditional expressions of the 'if' statements situated alongside each other are identical. Check lines: 72, 76. AWSSDK.Glue.Net45 UpdateConnectionResponseUnmarshaller.cs 72
    • V3029 The conditional expressions of the 'if' statements situated alongside each other are identical. Check lines: 123, 127. AWSSDK.Neptune.Net45 RestoreDBClusterFromSnapshotResponseUnmarshaller.cs 123
    • V3029 The conditional expressions of the 'if' statements situated alongside each other are identical. Check lines: 167, 171. AWSSDK.Neptune.Net45 RestoreDBClusterFromSnapshotResponseUnmarshaller.cs 167
    • V3029 The conditional expressions of the 'if' statements situated alongside each other are identical. Check lines: 127, 131. AWSSDK.RDS.Net45 RestoreDBClusterFromSnapshotResponseUnmarshaller.cs 127
    • V3029 The conditional expressions of the 'if' statements situated alongside each other are identical. Check lines: 171, 175. AWSSDK.RDS.Net45 RestoreDBClusterFromSnapshotResponseUnmarshaller.cs 171
    • V3029 The conditional expressions of the 'if' statements situated alongside each other are identical. Check lines: 99, 103. AWSSDK.Rekognition.Net45 RecognizeCelebritiesResponseUnmarshaller.cs 99

    What are you?

    PVS-Studio Warning: V3062 An object 'attributeName' is used. Consider checking the first actual argument of the 'Contains' method. AWSSDK.MobileAnalytics.Net45 CustomEvent.cs 261

    /// <summary>/// Dictionary that stores attribute for this event only./// </summary>private Dictionary<string,string> _attributes =
      new Dictionary<string,string>();
    /// <summary>/// Gets the attribute./// </summary>    /// <param name="attributeName">Attribute name.</param>/// <returns>The attribute. Return null of attribute doesn't///          exist.</returns>publicstringGetAttribute(string attributeName){
      if(string.IsNullOrEmpty(attributeName))
      {
        thrownew ArgumentNullException("attributeName");
      }
      string ret = null;
      lock(_lock)
      {
        if(attributeName.Contains(attributeName))  // <=
          ret = _attributes[attributeName];
      }
      return ret;
    }

    The analyzer detected an error in the GetAttribute method : the string is checked that it contains itself. From the description of the method it follows that if the attribute name ( attributeName key ) is found (in the _attributes dictionary ), then the attribute value should be returned, otherwise null . In fact, since the condition attributeName.Contains (attributeName) is always true, an attempt is made to return a value using a key that may not be found in the dictionary. Then, instead of returning null, KeyNotFoundException will be thrown .

    Let's try to fix this code. To better understand how to do this, you should look at another method:

    /// <summary>/// Determines whether this instance has attribute the specified/// attributeName./// </summary>/// <param name="attributeName">Attribute name.</param>/// <returns>Return true if the event has the attribute, else///          false.</returns>publicboolHasAttribute(string attributeName){
      if(string.IsNullOrEmpty(attributeName))
      {
        thrownew ArgumentNullException("attributeName");
      }
      bool ret = false;
      lock(_lock)
      {
        ret = _attributes.ContainsKey(attributeName);
      }
      return ret;
    }

    This method checks whether the attribute name ( attributeName key ) exists in the _attributes dictionary . Let's go back to the GetAttribute method and correct the error:

    publicstringGetAttribute(string attributeName){
      if(string.IsNullOrEmpty(attributeName))
      {
        thrownew ArgumentNullException("attributeName");
      }
      string ret = null;
      lock(_lock)
      {
        if(_attributes.ContainsKey(attributeName))
          ret = _attributes[attributeName];
      }
      return ret;
    }

    Now the method does exactly what is stated in the description.

    And one more small note to this code snippet. I noticed that authors use lock when working with the _attributes dictionary . It is clear that this is necessary for multi-threaded access, but the lock design is rather slow and cumbersome. Instead of a Dictionary in this case, it might be more convenient to use a thread-safe version of the dictionary - ConcurrentDictionary . Then the need for lock disappears. Although, maybe I do not know about some features of the project.

    Suspicious behavior

    Warning PVS-Studio: V3063[CWE-571] A part of conditional expression is always true if it is evaluated: string.IsNullOrEmpty (inferredIndexName). AWSSDK.DynamoDBv2.PCL ContextInternal.cs 802

    privatestaticstringGetQueryIndexName(....){
      ....
      string inferredIndexName = null;
      if (string.IsNullOrEmpty(specifiedIndexName) &&
          indexNames.Count == 1)
      {
        inferredIndexName = indexNames[0];
      }
      elseif (indexNames.Contains(specifiedIndexName,
                                   StringComparer.Ordinal))
      {
        inferredIndexName = specifiedIndexName;
      }
      elseif (string.IsNullOrEmpty(inferredIndexName) &&  // <=
               indexNames.Count > 0)
        thrownew InvalidOperationException("Local Secondary Index range
          key conditions are used but no index could be inferred from
          model. Specified index name = " + specifiedIndexName);
      ....
    }

    The analyzer alerted the verification of string.IsNullOrEmpty (inferredIndexName) . Indeed, the string inferredIndexName is assigned null , then the value of this variable does not change anywhere, and then for some reason it is checked for null or an empty string. It looks suspicious. Let's take a closer look at the above code snippet. I deliberately did not reduce it for a better understanding of the situation. So, in the first if statement (and also in the next), the specifiedIndexName variable is checked in some way . Depending on the result of the checks, the variable inferredIndexName gets a new value. Now pay attention to the third if statement. The body of this operator (throwing an exception) will be executed in the case of indexNames.Count> 0 , since the first part of the condition, string.IsNullOrEmpty (inferredIndexName), is always true. Perhaps the variables specifiedIndexName and inferredIndexName are simply confused . Or the third check should be without else , representing a stand-alone if statement :

    if (string.IsNullOrEmpty(specifiedIndexName) &&
        indexNames.Count == 1)
    {
      inferredIndexName = indexNames[0];
    }
    elseif (indexNames.Contains(specifiedIndexName,
                                 StringComparer.Ordinal))
    {
      inferredIndexName = specifiedIndexName;
    }
    if (string.IsNullOrEmpty(inferredIndexName) &&
        indexNames.Count > 0)
        thrownew InvalidOperationException(....);

    In this case, it is difficult to give a definite answer about the options for correcting this code. But it is definitely necessary for the author to inspect it.

    NullReferenceException

    PVS-Studio Warning: V3095 [CWE-476] The 'conditionValues' object was used against it. Check lines: 228, 238. AWSSDK.Core.Net45 JsonPolicyWriter.cs 228

    privatestaticvoidwriteConditions(....){
      ....
      foreach (....)
      {
        IList<string> conditionValues = keyEntry.Value;
        if (conditionValues.Count == 0) // <=continue;
        ....
        if (conditionValues != null && conditionValues.Count != 0)
        {
          ....
        }
        ....
      }
    }

    Classics of the genre. The conditionValues variable is used without first checking for null equality . In this case, further in the code, such verification is performed, but it is too late. :)

    The code can be corrected as follows:

    privatestaticvoidwriteConditions(....){
      ....
      foreach (....)
      {
        IList<string> conditionValues = keyEntry.Value;
        if (conditionValues != null && conditionValues.Count == 0)
          continue;
        ....
        if (conditionValues != null && conditionValues.Count != 0)
        {
          ....
        }
        ....
      }
    }

    A few more similar errors were found in the code.

    PVS-Studio warnings:

    • V3095 [CWE-476] The 'ts.Listeners' object was used against it. Check lines: 140, 143. AWSSDK.Core.Net45 Logger.Diagnostic.cs 140
    • V3095 [CWE-476] The 'obj' object has been verified against null. Check lines: 743, 745. AWSSDK.Core.Net45 JsonMapper.cs 743
    • V3095 [CWE-476] The 'multipartUpload MultipartUploadpartsList' object has been verified against null. Check lines: 65, 67. AWSSDK.S3.Net45 CompleteMultipartUploadRequestMarshaller.cs 65

    The following warning is very similar in meaning, but the situation is inverse of the one discussed above.

    PVS-Studio warning : V3125 [CWE-476] Check lines: 139, 127. AWSSDK.Core.Net45 RefreshingAWSCredentials.cs 139

    privatevoidUpdateToGeneratedCredentials(
      CredentialsRefreshState state){
      string errorMessage;
      if (ShouldUpdate)
      {  
        ....
        if (state == null)
          errorMessage = "Unable to generate temporary credentials";
        else
          ....
        thrownew AmazonClientException(errorMessage);
      }
      state.Expiration -= PreemptExpiryTime;  // <=
      ....
    }

    One of the code snippets contains a check for the value of the state variable for null equality . Further, the state variable is used to unsubscribe from the PreemptExpiryTime event , the test for null is no longer performed, and a NullReferenceException exception may be thrown . More secure code option:

    privatevoidUpdateToGeneratedCredentials(
      CredentialsRefreshState state){
      string errorMessage;
      if (ShouldUpdate)
      {  
        ....
        if (state == null)
          errorMessage = "Unable to generate temporary credentials";
        else
          ....
        thrownew AmazonClientException(errorMessage);
      }
      if (state != null)
        state.Expiration -= PreemptExpiryTime;
      ....
    }

    There are other similar errors in the code.

    PVS-Studio warnings:

    • V3125 [CWE-476] The 'wrappedRequest.Content' object was verified against null. Check lines: 395, 383. AWSSDK.Core.Net45 HttpHandler.cs 395
    • V3125 [CWE-476] The 'datasetUpdates' object was used after it was verified against null. Check lines: 477, 437. AWSSDK.CognitoSync.Net45 Dataset.cs 477
    • V3125 [CWE-476] The 'cORSConfigurationCORSConfigurationcORSRulesListValue' object was used after it was verified against null. Check lines: 125, 111. AWSSDK.S3.Net45 PutCORSConfigurationRequestMarshaller.cs 125
    • V3125 [CWE-476] The 'lifecycleConfigurationLifecycleConfigurationrulesListValue' object was used after it was verified against null. Check lines: 157, 68. AWSSDK.S3.Net45 PutLifecycleConfigurationRequestMarshaller.cs 157
    • V3125 [CWE-476] The 'this.Key' object was used after it was verified against null. Check lines: 199, 183. AWSSDK.S3.Net45 S3PostUploadRequest.cs 199

    No Alternative Reality

    PVS-Studio Warning: V3009 [CWE-393] This is the true value of 'true'. AWSSDK.Core.Net45 Lexer.cs 651

    privatestaticboolState19(....){
      while (....) {
        switch (....) {
        case'"':
          ....
          returntrue;
        case'\\':
          ....
          returntrue;
        default:
          ....
          continue;
        }
      }
      returntrue;
    }

    The method always returns true . Let's see how critical this is for the calling code. I tracked the use of the State19 method . It participates in filling the array of fsm_handler_table handlers along with other similar methods (there are 28 of them with names, respectively, from State1 to State28 ). Here it is important to note that in addition to State19 , warnings V3009 [CWE-393] were also issued for some other processors . These are handlers: State23, State26, State27, State28 . Warnings issued by the analyzer for them:

    • V3009 [CWE-393] This is the true value of this method. AWSSDK.Core.Net45 Lexer.cs 752
    • V3009 [CWE-393] This is the true value of this method. AWSSDK.Core.Net45 Lexer.cs 810
    • V3009 [CWE-393] This is the true value of this method. AWSSDK.Core.Net45 Lexer.cs 822
    • V3009 [CWE-393] This is the true value of this method. AWSSDK.Core.Net45 Lexer.cs 834

    Here is the declaration and initialization of the array of handlers:

    privatestatic StateHandler[] fsm_handler_table;
    ....
    privatestaticvoidPopulateFsmTables(){
      fsm_handler_table = new StateHandler[28] {
          State1,
          State2,
          ....
          State19,
          ....
          State23,
          ....
          State26,
          State27,
          State28
    };

    For completeness, let's look at the code of one of the handlers for which the analyzer had no complaints, for example, State2 :

    privatestaticboolState2(....){
      ....
      if (....) {
        returntrue;
      }
      switch (....) {
        ....
        default:
          returnfalse;
      }
    }

    And this is how callback handlers occur:

    publicboolNextToken(){
      ....
      while (true) {
        handler = fsm_handler_table[state - 1];
        if (! handler (fsm_context))  // <=thrownew JsonException (input_char);
        ....
      }
      ....
    }

    As you can see, the exception will be thrown if false is returned . In our case, for State19, State23, State26, State27, and State28 handlers, this will never happen. It looks suspicious. On the other hand, as many as five handlers have a similar behavior (they always return true ), so perhaps it was so intended and not the result of a typo.

    Why did I stop at all this in such detail? This situation is very indicative in the sense that a static analyzer is often only able to indicate a suspicious design. And even a person (not a machine) who does not have sufficient knowledge of the project, having spent time studying the code, is still not able to give an exhaustive answer about the presence of an error. The code must be studied by the developer.

    Pointless checks

    PVS-Studio warning : V3022 [CWE-571] Expression 'doLog' is always true. AWSSDK.Core.Net45 StoredProfileAWSCredentials.cs 235

    privatestaticboolValidCredentialsExistInSharedFile(....){
      ....
      var doLog = false;
      try
      {
        if (....)
        {
          returntrue;
        }
        else
        {
          doLog = true;
        }
      }
      catch (InvalidDataException)
      {
        doLog = true;
      }
      if (doLog)  // <=
      {
        ....
      }
      ....
    }

    Notice the doLog variable . After initialization to false , then in the code this variable will be set to true in all cases. Thus, the if (doLog) check is always true. Perhaps, earlier in this method there was a branch in which no value was assigned to the doLog variable , then at the time of the test it could contain the value false obtained during initialization. But now there is no such branch.

    Another similar error:

    Warning PVS-Studio: V3022 Expression '! Result' is always false. AWSSDK.CognitoSync.PCL SQLiteLocalStorage.cs 353

    publicvoidPutValue(....){
      ....
      bool result = PutValueHelper(....);
      if (!result) <=
      {
        _logger.DebugFormat("{0}",
          @"Cognito Sync - SQLiteStorage - Put Value Failed");
      }
      else
      {
        UpdateLastModifiedTimestamp(....);
      }
      ....
    }

    The analyzer states that the value of the result variable is always true . This is only possible if the PutValueHelper method always returns true . Let's look at this method:

    privateboolPutValueHelper(....){
      ....
      if (....))
      {
          returntrue;
      }
      if (record == null)
      {
        ....
        returntrue;
      }
      else
      {
        ....
        returntrue;
      }
    }

    Indeed, the method returns true under any condition. Moreover, the analyzer issued a warning for this method. PVS-Studio Warning: V3009 [CWE-393] This is the odd one. SQLiteLocalStorage.cs 1016

    I deliberately did not give this warning earlier when I considered other V3009 errors , but saved it for this case. Thus, the analyzer was right to point out the error V3022 in the calling code.

    Copy-Paste. Again

    Warning PVS-Studio: V3001There are identical sub-expressions "this.token == JsonToken.String" operator. AWSSDK.Core.Net45 JsonReader.cs 343

    publicboolRead(){
      ....
      if (
        (this.token == JsonToken.ObjectEnd ||
        this.token == JsonToken.ArrayEnd ||
        this.token == JsonToken.String ||  // <=this.token == JsonToken.Boolean ||
        this.token == JsonToken.Double ||
        this.token == JsonToken.Int ||
        this.token == JsonToken.UInt ||
        this.token == JsonToken.Long ||
        this.token == JsonToken.ULong ||
        this.token == JsonToken.Null ||
        this.token == JsonToken.String  // <=
        ))
      {
        ....
      }
      ....
    }

    Double compared field this.token the value JsonToken.String listing JsonToken . Probably one of the comparisons should contain a different enumeration value. If so, then a serious mistake was made.

    Refactoring + inattention?

    PVS-Studio warning : V3025 [CWE-685] Incorrect format. A different number of formatted items is expected while calling 'Format' function. Arguments not used: AWSConfigs.AWSRegionKey. AWSSDK.Core.Net45 AWSRegion.cs 116

    publicInstanceProfileAWSRegion(){
      ....
      if (region == null)
      {
        thrownew InvalidOperationException(
          string.Format(CultureInfo.InvariantCulture,
            "EC2 instance metadata was not available or did not contain 
              region information.",
            AWSConfigs.AWSRegionKey));
      }
      ....
    }

    Probably, the format string for the string.Format method previously contained the output specifier {0} , for which the AWSConfigs.AWSRegionKey argument was specified . Then the string was changed, the specifier disappeared, but the argument was forgotten to be deleted. The above code snippet works without errors (an exception would be thrown otherwise - a specifier without an argument), but it looks ugly. The code should be corrected as follows:

    if (region == null)
    {
      thrownew InvalidOperationException(
        "EC2 instance metadata was not available or did not contain 
          region information.");
    }

    Unsafe

    PVS-Studio Warning: V3083 [CWE-367] Unsafe invocation of event 'mOnSyncSuccess', NullReferenceException is possible. Consider assigning an event to a local variable before invoking it. AWSSDK.CognitoSync.PCL Dataset.cs 827

    protectedvoidFireSyncSuccessEvent(List<Record> records){
      if (mOnSyncSuccess != null)
      {
        mOnSyncSuccess(this, new SyncSuccessEventArgs(records));
      }
    }

    A fairly common situation is an unsafe invocation of an event handler. Between checking the mOnSyncSuccess variable for equality to null and calling the handler, you can unsubscribe from the event, and its value becomes zero. The probability of such a scenario is small, but it is still better to make the code more secure:

    protectedvoidFireSyncSuccessEvent(List<Record> records){
      mOnSyncSuccess?.Invoke(this, new SyncSuccessEventArgs(records));
    }

    There are other similar errors in the code.

    PVS-Studio warnings:

    • V3083 [CWE-367] Unsafe invocation of event 'mOnSyncFailure', NullReferenceException is possible. Consider assigning an event to a local variable before invoking it. AWSSDK.CognitoSync.PCL Dataset.cs 839
    • V3083 [CWE-367] Unsafe invocation of event, NullReferenceException is possible. Consider assigning an event to a local variable before invoking it. AWSSDK.Core.PCL AmazonServiceClient.cs 332
    • V3083 [CWE-367] Unsafe invocation of event, NullReferenceException is possible. Consider assigning event to a local variable before invoking it. AWSSDK.Core.PCL AmazonServiceClient.cs 344
    • V3083 [CWE-367] Unsafe invocation of event, NullReferenceException is possible. Consider assigning event to a local variable before invoking it. AWSSDK.Core.PCL AmazonServiceClient.cs 357
    • V3083 [CWE-367] Unsafe invocation of event 'mExceptionEvent', NullReferenceException is possible. Consider assigning event to a local variable before invoking it. AWSSDK.Core.PCL AmazonServiceClient.cs 366
    • V3083 [CWE-367] Unsafe invocation of event, NullReferenceException is possible. Consider assigning event to a local variable before invoking it. AWSSDK.Core.PCL AmazonWebServiceRequest.cs 78
    • V3083 [CWE-367] Unsafe invocation of event 'OnRead', NullReferenceException is possible. Consider assigning event to a local variable before invoking it. AWSSDK.Core.PCL EventStream.cs 97
    • V3083 [CWE-367] Unsafe invocation of event, NullReferenceException is possible. Consider assigning event to a local variable before invoking it. AWSSDK.Core.Android NetworkReachability.cs 57
    • V3083 [CWE-367] Unsafe invocation of event, NullReferenceException is possible. Consider assigning event to a local variable before invoking it. AWSSDK.Core.Android NetworkReachability.cs 94
    • V3083 [CWE-367] Unsafe invocation of event, NullReferenceException is possible. Consider assigning event to a local variable before invoking it. AWSSDK.Core.iOS NetworkReachability.cs 54

    Incomplete

    PVS-Studio warning class : V3126 Type 'JsonData' implementing IEquatable <T> interface does not override the 'GetHashCode' method. AWSSDK.Core.Net45 JsonData.cs 26

    publicclassJsonData : IJsonWrapper, IEquatable<JsonData>
    {
      ....
    }

    The JsonData class contains quite a lot of code, so I didn’t cite it as a whole, confining myself to the declaration. This class really does not contain the overridden method GetHashCode , which is unsafe, as it can lead to erroneous behavior when using the JsonData type to work, for example, with collections. Probably, at present there is no problem, but in the future the strategy of using this type may change. This error is described in more detail in the documentation .

    Conclusion

    These are all the interesting errors that I managed to find in the AWS SDK code for .NET using the PVS-Studio static analyzer. Once again I will emphasize the quality of the project. I found a very small number of errors for 5 million lines of code. Although it is likely that a more thorough analysis of the warnings issued would allow me to add a few more errors to this list. But it is also very likely that I have in vain ranked some of the described warnings as errors. In this case, only the developer who is in the context of the code being verified always makes unambiguous conclusions.



    If you want to share this article with an English-speaking audience, then please use the link to the translation: Sergey Khrenov. Amazon Web Services SDK source code for .NET

    Also popular now: