# Examining suspicious code fragments in AWS SDK for \.NET
Today we are dissecting AWS SDK for \.NET\. We will look at suspicious code fragments, figure out what's wrong with them, and try to reproduce some of the errors\. Make yourself a cup of coffee and get cozy\.
## Some analysis details
**What project is it?**
AWS\.SDK for \.NET enables \.NET developers to work with Amazon Web Services, Amazon S3, Amazon DynamoDB, etc\.
I've taken the source code from the [GitHub](https://github.com/aws/aws-sdk-net) page of the project\. If you need the exact version, here's the commit SHA: 93a94821dc8ff7a0073b74def6549728da3b51c7\.
**What tools were used to check the project?**
I checked the code with the [PVS\-Studio](https://pvs-studio.com/en/pvs-studio/) analyzer using the plugin for Visual Studio\.
**What else is there to say?**
Some warnings may look familiar to you, as those code fragments haven't changed since the [last check](https://pvs-studio.com/en/blog/posts/csharp/0605/)\. In this article, I duplicated the warnings that I found interesting\.
Enough with the check details, let's take a look at the suspicious code fragments\.
## Examining suspicious code fragments
**Issue \#1**
```cpp
public static object GetAttr(object value, string path)
{
if (string.IsNullOrEmpty(path)) throw new ArgumentNullException("path");
var parts = path.Split('.');
var propertyValue = value;
for (int i = 0; i < parts.Length; i++)
{
var part = parts[i];
// indexer is always at the end of path e.g. "Part1.Part2[3]"
if (i == parts.Length - 1)
{
....
// indexer detected
if (indexerStart >= 0)
{
....
if (!(propertyValue is IList))
throw
new ArgumentException("Object addressing by pathing segment '{part}'
with indexer must be IList");
....
}
}
if (!(propertyValue is IPropertyBag))
throw
new ArgumentException("Object addressing by pathing segment '{part}'
must be IPropertyBag");
....
}
....
}
```
GitHub links: [\#1](https://github.com/aws/aws-sdk-net/blob/cae0a7b336f1d3d2fdae653095f64a3df82f3cc1/sdk/src/Core/Amazon.Runtime/Internal/Endpoints/StandardLibrary/Fn.cs#L82), [\#2](https://github.com/aws/aws-sdk-net/blob/cae0a7b336f1d3d2fdae653095f64a3df82f3cc1/sdk/src/Core/Amazon.Runtime/Internal/Endpoints/StandardLibrary/Fn.cs#L93)\.
The PVS\\\-Studio warning — V3138
String literal contains potential interpolated expression\. Consider inspecting: part\. Fn\.cs 82
String literal contains potential interpolated expression\. Consider inspecting: part\. Fn\.cs 93
It looks like developers forgot to interpolate the exception messages\. So, the _\{part\}_ string literal will be used instead of the actual value of the _part_ variable\.
**Issue \#2**
```cpp
private CredentialsRefreshState Authenticate(ICredentials userCredential)
{
....
ICoreAmazonSTS coreSTSClient = null;
try
{
....
coreSTSClient =
ServiceClientHelpers.CreateServiceFromAssembly(....);
}
catch (Exception e)
{
....
}
var samlCoreSTSClient
#if NETSTANDARD
= coreSTSClient as ICoreAmazonSTS_SAML;
if (coreSTSClient == null)
{
throw new NotImplementedException(
"The currently loaded version of AWSSDK.SecurityToken
doesn't support SAML authentication.");
}
#else
= coreSTSClient;
#endif
try
{
var credentials = samlCoreSTSClient.CredentialsFromSAMLAuthentication(....);
}
catch (Exception e)
{
var wrappedException =
new AmazonClientException("Credential generation from
SAML authentication failed.",
e);
var logger = Logger.GetLogger(typeof(FederatedAWSCredentials));
logger.Error(wrappedException, wrappedException.Message);
throw wrappedException;
}
....
}
```
The [GitHub link](https://github.com/aws/aws-sdk-net/blob/cae0a7b336f1d3d2fdae653095f64a3df82f3cc1/sdk/src/Core/Amazon.Runtime/Credentials/FederatedAWSCredentials.cs#L219)\.
The PVS\\\-Studio warning — V3019
Possibly an incorrect variable is compared to null after type conversion using 'as' keyword\. Check variables 'coreSTSClient', 'samlCoreSTSClient'\. FederatedAWSCredentials\.cs 219
We need this large code fragment to understand the context better\. The error lurks here:
```cpp
var samlCoreSTSClient
#if NETSTANDARD
= coreSTSClient as ICoreAmazonSTS_SAML;
if (coreSTSClient == null)
{
throw new NotImplementedException(
"The currently loaded version of AWSSDK.SecurityToken
doesn't support SAML authentication.");
}
```
In the _if_ statement condition, the wrong variable is checked for _null_\. The _samlCoreSTSClient_ variable should have been checked instead of coreSTSClient\.
Let's take a look at the following elements:
* _samlCoreSTSClient_ is the name of the resulting variable;
* _ICoreAmazonSTS\_SAML_ is the type of an interface being cast to;
* _"\.\.\. doesn't support SAML authentication"_ is the text of the exception message\.
SAML is mentioned everywhere except for the name of the variable being checked \(_coreSTSClient_\)\. :\)
It's interesting how checking different variables changes the logic if the casting fails\.
When checking _samlCoreSTSClient_:
* \-\> casting with the _as_ operator
* \-\> checking if _samlCoreSTSClient_ equals _null_
* _\-\> _throwing _NotImplementedException_
When checking _coreSTSClient_:
* \-\> casting with the _as_ operator
* \-\> checking if _coreSTSClient_ equals _null_
* _\-\> _attempting to call the _CredentialsFromSAMLAuthentication_ method
* \-\> throwing the _NotImplementedException_ exception
* \-\> catching an exception in _catch_
* \-\> logging the issue
* \-\> throwing _AmazonClientException_
That is, an exception of a different type and with a different message will be thrown in the external code\.
By the way, checking for the wrong variable after using the _as_ operator is a quite common error in C\# projects\. [Take a look at other examples](https://pvs-studio.com/en/blog/examples/v3019/)\.
**Issue \#3**
```cpp
public static class EC2InstanceMetadata
{
[Obsolete("EC2_METADATA_SVC is obsolete, refer to ServiceEndpoint
instead to respect environment and profile overrides.")]
public static readonly string EC2_METADATA_SVC = "http://169.254.169.254";
[Obsolete("EC2_METADATA_ROOT is obsolete, refer to EC2MetadataRoot
instead to respect environment and profile overrides.")]
public static readonly string
EC2_METADATA_ROOT = EC2_METADATA_SVC + LATEST + "/meta-data";
[Obsolete("EC2_USERDATA_ROOT is obsolete, refer to EC2UserDataRoot
instead to respect environment and profile overrides.")]
public static readonly string
EC2_USERDATA_ROOT = EC2_METADATA_SVC + LATEST + "/user-data";
[Obsolete("EC2_DYNAMICDATA_ROOT is obsolete, refer to EC2DynamicDataRoot
instead to respect environment and profile overrides.")]
public static readonly string
EC2_DYNAMICDATA_ROOT = EC2_METADATA_SVC + LATEST + "/dynamic";
[Obsolete("EC2_APITOKEN_URL is obsolete, refer to EC2ApiTokenUrl
instead to respect environment and profile overrides.")]
public static readonly string
EC2_APITOKEN_URL = EC2_METADATA_SVC + LATEST + "/api/token";
public static readonly string
LATEST = "/latest",
AWS_EC2_METADATA_DISABLED = "AWS_EC2_METADATA_DISABLED";
....
}
```
The [GitHub link](https://github.com/aws/aws-sdk-net/blob/cae0a7b336f1d3d2fdae653095f64a3df82f3cc1/sdk/src/Core/Amazon.Util/EC2InstanceMetadata.cs#L57)\.
The PVS\\\-Studio warning — V3070
Uninitialized variable 'LATEST' is used when initializing the 'EC2\_METADATA\_ROOT' variable\. EC2InstanceMetadata\.cs 57
Uninitialized variable 'LATEST' is used when initializing the 'EC2\_USERDATA\_ROOT' variable\. EC2InstanceMetadata\.cs 60
Uninitialized variable 'LATEST' is used when initializing the 'EC2\_DYNAMICDATA\_ROOT' variable\. EC2InstanceMetadata\.cs 63
Uninitialized variable 'LATEST' is used when initializing the 'EC2\_APITOKEN\_URL' variable\. EC2InstanceMetadata\.cs 66
Note the order in which the fields are declared and initialized\.
The _EC2\_APITOKEN\_URL_, _EC2\_DYNAMICDATA\_ROOT_, _EC2\_USERDATA\_ROOT_, and _EC2\_METADATA\_ROOT_ fields are declared first\. Each of them uses the _LATEST_ field in the initializer\. However, the field is not yet initialized when being used, because it is declared further in the code\. As a result, when calculating values for the _EC2\_\*_ fields, the _default\(string\)_ value \(_null_\) will be used instead of the _"/latest"_ string\.
We can easily verify the above by referring to the corresponding API:
```cpp
var arr = new[]
{
EC2InstanceMetadata.EC2_APITOKEN_URL,
EC2InstanceMetadata.EC2_DYNAMICDATA_ROOT,
EC2InstanceMetadata.EC2_USERDATA_ROOT,
EC2InstanceMetadata.EC2_METADATA_ROOT
};
foreach(var item in arr)
Console.WriteLine(item);
```
The result of code execution:
As you can see, no line has the _"/latest"_ literal\.
But it's debatable if this is a mistake\. The order of field initialization was changed in an [individual commit](https://github.com/aws/aws-sdk-net/commit/8cf5524d5a5cb2b6749c3d1e465770390e420a13)\. In the same commit, the fields were decorated with the _Obsolete_ attribute\. Although, if you are not going to use the actual _LATEST_ value, it's better not to use it at all\. This way, the code won't confuse anybody\.
**Issue \#4**
```cpp
public IRequest Marshall(GetObjectTorrentRequest getObjectTorrentRequest)
{
IRequest request = new DefaultRequest(getObjectTorrentRequest, "AmazonS3");
request.HttpMethod = "GET";
if (getObjectTorrentRequest.IsSetRequestPayer())
request.Headers
.Add(S3Constants.AmzHeaderRequestPayer,
S3Transforms.ToStringValue(getObjectTorrentRequest.RequestPayer
.ToString()));
if (getObjectTorrentRequest.IsSetRequestPayer())
request.Headers
.Add(S3Constants.AmzHeaderRequestPayer,
S3Transforms.ToStringValue(getObjectTorrentRequest.RequestPayer
.ToString()));
if (getObjectTorrentRequest.IsSetExpectedBucketOwner())
request.Headers
.Add(S3Constants.AmzHeaderExpectedBucketOwner,
S3Transforms.ToStringValue(
getObjectTorrentRequest.ExpectedBucketOwner));
....
}
```
The [GitHub link](https://github.com/aws/aws-sdk-net/blob/cae0a7b336f1d3d2fdae653095f64a3df82f3cc1/sdk/src/Services/S3/Custom/Model/Internal/MarshallTransformations/GetObjectTorrentRequestMarshaller.cs#L43)\.
The PVS\\\-Studio warning — V3029
The conditional expressions of the 'if' statements situated alongside each other are identical\. Check lines: 41, 43\. GetObjectTorrentRequestMarshaller\.cs 41
The first two _if_ statements completely duplicate each other in both conditions and bodies\. Either one of them is redundant and needs to be removed, or one of the statements should have a different condition and perform other actions\.
**Issue \#5**
```cpp
public string Region
{
get
{
if (String.IsNullOrEmpty(this.linker.s3.region))
{
return "us-east-1";
}
return this.linker.s3.region;
}
set
{
if (String.IsNullOrEmpty(value))
{
this.linker.s3.region = "us-east-1";
}
this.linker.s3.region = value;
}
}
```
The [GitHub link](https://github.com/aws/aws-sdk-net/blob/cae0a7b336f1d3d2fdae653095f64a3df82f3cc1/sdk/src/Services/DynamoDBv2/Custom/DataModel/S3Link.cs#L114)\.
The PVS\\\-Studio warning — V3008
The 'this\.linker\.s3\.region' variable is assigned values twice successively\. Perhaps this is a mistake\. Check lines: 116, 114\. S3Link\.cs 116
The code above is quite interesting\. On the one hand, there is an error\. On the other hand, the error will not impact the app's logic if we work only with the _Region_ property\.
The error itself lurks in the _set_ accessor\. _value_ will always be written to the _this\.linker\.s3\.region_ property\. So, the _String\.IsNullOrEmpty\(value\)_ check has no effect\. There is also a check in the _get_ accessor: if _linker\.s3\.region_ is _null_ or an empty string, the property returns the _"us\-east\-1"_ value\.
So, here's what happens:_ _it makes no difference whether there is an issue or not for a user who just deals with the _Region_ property\. Although, it's better to fix the error anyway\.
**Issue \#6**
```cpp
internal string
GetPreSignedURLInternal(....)
{
....
RegionEndpoint endpoint = RegionEndpoint.GetBySystemName(region);
var s3SignatureVersionOverride
= endpoint.GetEndpointForService("s3",
Config.ToGetEndpointForServiceOptions())
.SignatureVersionOverride;
if (s3SignatureVersionOverride == "4" || s3SignatureVersionOverride == null)
{
signatureVersionToUse = SignatureVersion.SigV4;
}
var fallbackToSigV2 = useSigV2Fallback && !AWSConfigsS3.UseSigV4SetExplicitly;
if ( endpoint?.SystemName == RegionEndpoint.USEast1.SystemName
&& fallbackToSigV2)
{
signatureVersionToUse = SignatureVersion.SigV2;
}
....
}
```
The [GitHub link](https://github.com/aws/aws-sdk-net/blob/cae0a7b336f1d3d2fdae653095f64a3df82f3cc1/sdk/src/Services/S3/Custom/AmazonS3Client.Extensions.cs#L111)\.
The PVS\\\-Studio warning — V3095
The 'endpoint' object was used before it was verified against null\. Check lines: 111, 118\. AmazonS3Client\.Extensions\.cs 111
A strange order of handling potential _null_ values attracts bugs\. Sometimes the value is used before it is checked for _null_\. This is where we encounter puzzles: is it an error and an exception will be thrown? Is the check redundant, and the variable can't be _null_? Is it something else\.\.\.?
Here we have a similar issue\. Developers accessed the _endpoint_ variable unconditionally \(_endpoint\.GetEndpointForService_\), but then they used the conditional access operator \(_endpoint?\.SystemName_\)\.
**Issue \#7**
```cpp
public class GetObjectMetadataResponse : AmazonWebServiceResponse
{
....
private ServerSideEncryptionMethod
serverSideEncryption;
private ServerSideEncryptionCustomerMethod
serverSideEncryptionCustomerMethod;
....
public ServerSideEncryptionCustomerMethod
ServerSideEncryptionCustomerMethod
{
get
{
if (this.serverSideEncryptionCustomerMethod == null)
return ServerSideEncryptionCustomerMethod.None;
return this.serverSideEncryptionCustomerMethod;
}
set { this.serverSideEncryptionCustomerMethod = value; }
}
// Check to see if ServerSideEncryptionCustomerMethod property is set
internal bool IsSetServerSideEncryptionCustomerMethod()
{
return this.serverSideEncryptionCustomerMethod != null;
}
....
public ServerSideEncryptionMethod
ServerSideEncryptionMethod
{
get
{
if (this.serverSideEncryption == null)
return ServerSideEncryptionMethod.None;
return this.serverSideEncryption;
}
set { this.serverSideEncryption = value; }
}
// Check to see if ServerSideEncryptionCustomerMethod property is set
internal bool IsSetServerSideEncryptionMethod()
{
return this.serverSideEncryptionCustomerMethod != null;
}
....
}
```
GitHub links: [\#1](https://github.com/aws/aws-sdk-net/blob/cae0a7b336f1d3d2fdae653095f64a3df82f3cc1/sdk/src/Services/S3/Custom/Model/GetObjectMetadataResponse.cs#L311), [\#2](https://github.com/aws/aws-sdk-net/blob/cae0a7b336f1d3d2fdae653095f64a3df82f3cc1/sdk/src/Services/S3/Custom/Model/GetObjectMetadataResponse.cs#L334)\.
The PVS\\\-Studio warning — V3013
It is odd that the body of 'IsSetServerSideEncryptionMethod' function is fully equivalent to the body of 'IsSetServerSideEncryptionCustomerMethod' function\. GetObjectMetadataResponse\.cs 311
Let me warn you: similar names are about to make your eyes dazzled\. I guess that's what caused an error\.
The_ ServerSideEncryptionMethod _and the_ ServerSideEncryptionCustomerMethod _properties are defined in the _GetObjectMetadataResponse_ type\. They use the _serverSideEncryption_ and _serverSideEncryptionCustomerMethod_ backing fields:
* _ServerSideEncryptionMethod_ \-\>_ serverSideEncryption;_
* _ServerSideEncryptionCustomerMethod_ \-\>_ serverSideEncryptionCustomerMethod\._
There are _IsSetServerSideEncryptionMethod_ and _IsSetServerSideEncryptionCustomerMethod_ as well\. You may assume, they also use the _serverSideEncryption_ and _serverSideEncryptionCustomerMethod_ backing fields, respectively\.\.\. But no\. Because of the error, both methods check the _serverSideEncryptionCustomerMethod_ field\.
```cpp
// Check to see if ServerSideEncryptionCustomerMethod property is set
internal bool IsSetServerSideEncryptionCustomerMethod()
{
return this.serverSideEncryptionCustomerMethod != null;
}
// Check to see if ServerSideEncryptionCustomerMethod property is set
internal bool IsSetServerSideEncryptionMethod()
{
return this.serverSideEncryptionCustomerMethod != null;
}
```
The _IsSetServerSideEncryptionMethod_ method should check the _serverSideEncryption_ field\.
**Issue \#8**
```cpp
public string GetDecryptedPassword(string rsaPrivateKey)
{
RSAParameters rsaParams;
try
{
rsaParams = new PemReader(
new StringReader(rsaPrivateKey.Trim())
).ReadPrivatekey();
}
catch (Exception e)
{
throw new AmazonEC2Exception("Invalid RSA Private Key", e);
}
RSACryptoServiceProvider rsa = new RSACryptoServiceProvider();
rsa.ImportParameters(rsaParams);
byte[] encryptedBytes = Convert.FromBase64String(this.PasswordData);
var decryptedBytes = rsa.Decrypt(encryptedBytes, false);
string decrypted = Encoding.UTF8.GetString(decryptedBytes);
return decrypted;
}
```
The [GitHub link](https://github.com/aws/aws-sdk-net/blob/cae0a7b336f1d3d2fdae653095f64a3df82f3cc1/sdk/src/Services/EC2/Custom/Model/GetPasswordDataResponse.Extensions.cs#L48)\.
The PVS\\\-Studio warning — V3114
IDisposable object 'rsa' is not disposed before method returns\. GetPasswordDataResponse\.Extensions\.cs 48
The _RSACryptoServiceProvider_ type implements the _IDisposable_ interface\. In this code, however, the _Dispose_ method is called neither explicitly nor implicitly\.
I can't say if it's critical in this case\. However, it would be better to call _Dispose_ to clean up data, especially when the code is working with passwords, etc\.
**Issue \#9**
```cpp
public class ResizeJobFlowStep
{
....
public OnFailure? OnFailure
{
get { return this.OnFailure; }
set { this.onFailure = value; }
}
....
}
```
The [GitHub link](https://github.com/aws/aws-sdk-net/blob/cae0a7b336f1d3d2fdae653095f64a3df82f3cc1/sdk/src/Services/ElasticMapReduce/Custom/Model/ResizeJobFlowStep.cs#L171)\.
The PVS\\\-Studio warning — V3010
Possible infinite recursion inside 'OnFailure' property\. ResizeJobFlowStep\.cs 171
Due to a typo in the _get_ accessor of the _OnFailure_ property, the _OnFailure_ property is used instead of the _onFailure_ backing field\. An attempt to get a property value results in an infinite recursion, which causes _StackOverflowException_\.
We can easily reproduce this error by using the corresponding API:
```cpp
ResizeJobFlowStep obj = new ResizeJobFlowStep();
_ = obj.OnFailure;
```
Compile the code, run it, and get the expected result:
**Issue \#10**
```cpp
private static void
writeConditions(Statement statement, JsonWriter generator)
{
....
IList conditionValues = keyEntry.Value;
if (conditionValues.Count == 0)
continue;
generator.WritePropertyName(keyEntry.Key);
if (conditionValues.Count > 1)
{
generator.WriteArrayStart();
}
if (conditionValues != null && conditionValues.Count != 0)
{
foreach (string conditionValue in conditionValues)
{
generator.Write(conditionValue);
}
}
....
}
```
The [GitHub link](https://github.com/aws/aws-sdk-net/blob/cae0a7b336f1d3d2fdae653095f64a3df82f3cc1/sdk/src/Core/Amazon.Auth/AccessControlPolicy/Internal/JsonPolicyWriter.cs#L233)\.
The PVS\\\-Studio warning — V3095
The 'conditionValues' object was used before it was verified against null\. Check lines: 233, 238\. JsonPolicyWriter\.cs 233
The code looks weird: first, the reference from the _conditionValues_ variable is dereferenced, and then it is checked for _null_\. However, the value of the variable doesn't change\. So, if the reference is _null_, _NullReferenceException_ will occur while executing _conditionValues\.Count \=\= 0_\.
This code may have both an error and a redundant check for _null_ inequality\.
There is one thing I'd like to point out\. I got the impression that the project developers like to add _null_ equality checks just in case\. :\) Take a look at some examples below\.
```cpp
string[] settings
= value.Split(validSeparators, StringSplitOptions.RemoveEmptyEntries);
if (settings == null || settings.Length == 0)
return LoggingOptions.None;
```
The [GitHub link](https://github.com/aws/aws-sdk-net/blob/6fb0a41e7a4d4ba5e6cd9cf947beb65f06d8e58b/sdk/src/Core/AWSConfigs.cs#L278)\.
The _String\.Split_ method doesn't return _null_\. There's a similar check [here](https://github.com/aws/aws-sdk-net/blob/6fb0a41e7a4d4ba5e6cd9cf947beb65f06d8e58b/sdk/src/Core/Amazon.Util/AWSSDKUtils.cs#L587)\.
Here is another example of a similar check:
```cpp
var constructors
= GetConstructors(objectTypeWrapper, validConstructorInputs).ToList();
if (constructors != null && constructors.Count > 0)
```
The [GitHub link](https://github.com/aws/aws-sdk-net/blob/6fb0a41e7a4d4ba5e6cd9cf947beb65f06d8e58b/sdk/src/Services/DynamoDBv2/Custom/DataModel/Utils.cs#LL278C16-L278C16)\.
The _Enumerable\.ToList_ method doesn't return _null_, so the value of the _constructors_ variable can never be _null_\.
The example below is closer to the original one — developers first dereferenced the reference, then checked its value for _null_:
```cpp
TraceSource ts = new TraceSource(testName, sourceLevels);
ts.Listeners.AddRange(AWSConfigs.TraceListeners(testName));
// no listeners? skip
if (ts.Listeners == null || ts.Listeners.Count == 0)
```
The [GitHub link](https://github.com/aws/aws-sdk-net/blob/6fb0a41e7a4d4ba5e6cd9cf947beb65f06d8e58b/sdk/src/Core/Amazon.Runtime/Internal/Util/Logger.Diagnostic.cs#L143)\.
Although, I haven't found any cases where the _Listeners_ property could be _null_\. In \.NET, the return value of the property is marked with a null\-forgiving operator \([link to GitHub](https://github.com/dotnet/runtime/blob/3181f9c925ba65a7bbab0dc310a8abc1e3bfe68e/src/libraries/System.Diagnostics.TraceSource/src/System/Diagnostics/TraceSource.cs#L523)\):
```cpp
public TraceListenerCollection Listeners
{
get
{
Initialize();
return _listeners!;
}
}
```
**Issue \#11**
```cpp
private static string GetXamarinInformation()
{
var xamarinDevice = Type.GetType("Xamarin.Forms.Device, Xamarin.Forms.Core");
if (xamarinDevice == null)
{
return null;
}
var runtime = xamarinDevice.GetProperty("RuntimePlatform")
?.GetValue(null)
?.ToString() ?? "";
var idiom = xamarinDevice.GetProperty("Idiom")
?.GetValue(null)
?.ToString() ?? "";
var platform = runtime + idiom;
if (string.IsNullOrEmpty(platform))
{
platform = UnknownPlatform;
}
return string.Format(CultureInfo.InvariantCulture, "Xamarin_{0}", "Xamarin");
}
```
The [GitHub link](https://github.com/aws/aws-sdk-net/blob/cae0a7b336f1d3d2fdae653095f64a3df82f3cc1/sdk/src/Core/Amazon.Util/Internal/_netstandard/InternalSDKUtils.netstandard.cs#L70)\.
The PVS\\\-Studio warning — V3137
The 'platform' variable is assigned but is not used by the end of the function\. InternalSDKUtils\.netstandard\.cs 70
The last line of the method looks odd\. The _"Xamarin"_ string literal is substituted in the _"Xamarin\_\{0\}"_ template using _String\.Format\._ The value of the _platform_ variable, which can store the necessary information, is ignored\. That's strange\.
I can assume that the _return_ statement should look like this:
```cpp
return string.Format(CultureInfo.InvariantCulture, "Xamarin_{0}", platform);
```
By the way, there is a similar method for getting the Unity game engine information\. It is written in a similar pattern, but the return value is produced correctly:
```cpp
private static string GetUnityInformation()
{
var unityApplication
= Type.GetType("UnityEngine.Application, UnityEngine.CoreModule");
if (unityApplication == null)
{
return null;
}
var platform = unityApplication.GetProperty("platform")
?.GetValue(null)
?.ToString() ?? UnknownPlatform;
return string.Format(CultureInfo.InvariantCulture, "Unity_{0}", platform);
}
```
## Conclusion
Before publishing this article, I've already notified the developers of the issues I found in the project — here is the [link to the bug report](https://github.com/aws/aws-sdk-net/issues/2627)\.
Do you want to know if your project has similar issues? Check your code with the PVS\-Studio analyzer\.
[](https://pvs-studio.com/en/pvs-studio-download/)