ABCDEFGHIJKLMNOPQRSTUVWXYZ
1
Code Review Information
2
NameNitav Shah
3
UMD Email ID
nshah117@umd.edu
4
Developers
Murat Cakir, Michael Herzog, Marcus Gesing
5
ApplicationSmartstore
6
Application Code Location
https://github.com/smartstore/Smartstore.git
7
Application Code Commit ID (if applicable)
1efd10ea93a25734fab6dd971ccfe0a7076a31f2
8
Analysis Tools Used
Security Code Scan .NET
SonarQube
9
Date Last Updated4/14/26 11:01 am
10
11
12
13
Code Review Findings
14
No.CWE-ID
Finding Category
CriticalityFilename
Line Number: Position
DescriptionTechnical ImpactPotential MitigationComments
15
1CWE-916CryptographyHighSmartstore.Core/Platform/Security/Services/Encryptor.cs25In the Smartstore.Core Encryptor service, for hashing the password the developer is using the SHA1 hashing algorithm. The SHA-1 hashing algorithm is considered a weak hash or outdated hashing algorithm. SHA-1 hashing algorithm doesn't require sufficient computational power to be cracked. This acts as a major vulnerability in the Access Control of the application which can violate the confidentiality of sensitive data. NIST deprecated the use of SHA-1 in 2011. A weak hash can have a serious technical impact as if the password hashes are accessed by an attacker, they can be cracked using a brute force attack with comparatively less amount of effort. The near future proof mitigation would be to update the hashing algorithm from SHA-1 to a more strong and slow hashing algorithm like Argon2 or Bcrypt.
16
2CWE-328CryptographyHighSmartstore.Core/Platform/Security/Services/Encryptor.cs25In the Smartstore.Core Encryptor service, for hashing the password the developer is using the SHA1 hashing algorithm. The SHA-1 hashing algorithm is considered a weak hash or outdated hashing algorithm. SHA-1 hashing algorithm doesn't require sufficient computational power to be cracked. This acts as a major vulnerability in the Access Control of the application which can violate the confidentiality of sensitive data. NIST deprecated the use of SHA-1 in 2011. A weak hash can have a serious technical impact as if the password hashes are accessed by an attacker, they can be cracked using a brute force attack with comparatively less amount of effort.The near future proof mitigation would be to update the hashing algorithm from SHA-1 to a more strong and slow hashing algorithm like Argon2 or Bcrypt.
17
3CWE-327CryptographyHIghSmartstore.Core/Platform/Security/Services/Encryptor.cs66The developer is using TripleDES for symmetric encryption in the Encryptor Service. TripleDES is considered a retired algorithm by NIST. Moving forward usage of TripleDES for encryption can be considered risky. This vulnerability can lead to the compromise of sensitive data of the Smart store.The technical impact of this vulnerability is that to protect mission-critical data it is required to have a strong encryption algorithm that cannot be broken. But because of a risky encryption algorithm like 3DES sensitive data may be compromised such as credit card data. This vulnerability can be mitigated by using a more secure encryption algorithm like the AES.
18
4CWE-352AuthenticationLowSmartstore.Web/Controllers/CatalogController.cs587In the AddProductToCompareListAjax POST method the developer is not checking the Antiforgery Token. The method takes the id of the product as input which can be used in a CSRF attack. A malicious link can be used to add any product id to the compare list when clicked upon by authenticated users. The function is contained and can only add the products to the Product Compare List which is limited.The Antiforgery token shouldn't be ignored and should be validated before performing the method.
19
5CWE-352AuthenticationLowSmartstore.Web/Controllers/ProductController.cs246In the BackInStockSubscribePopup method the developer is not checking the Antiforgery Token. The method takes the id of the product as input which can be used in a CSRF attack. This method is called in an AJAX call which can be manipulated or converted to a malicious link. Server-side authentication is required for this call. As the function gives a popup to users about products back in stock, if a user repeats the malicious request multiple times they might have to handle unpredictable behavior from the application. The Antiforgery token shouldn't be ignored and should be validated before performing the method.
20
6CWE-352AuthenticationLowSmartstore.Web/Controllers/ProductController.cs546In the SetReviewHelpfulness method the developer is not checking the Antiforgery Token. The method takes the id of the product review and a boolean value of whether the product review was helpful for the user or not as input which can be used in a CSRF attack. A modified link can be used to consider a product review helpful for the victim. If this vulnerability is abused, product reviews can be boosted for fake reviews of a product if the malicious link is clicked by any user. The Antiforgery token shouldn't be ignored and should be validated before performing the method.
21
7CWE-521AuthenticationCriticalSmartstore.Core/Platform/Identity/Settings/CustomerSettings.cs45, 65The application only requires 6 digits as the minimum password length. The application also only requires 1 unique character among those 6 digits. The password is not sufficiently complex and can be cracked without a lot of computational effort. There is no check setup to check if the common password or the username is being used as a password or not. Technically if the password hashes are exposed they can be easily cracked because of the less number of possibilities of password due to the low password length requirement.Increase the minimum password length requirement to 12 with 5 unique characters. The application does require lower case, upper case, digit, and non-alphanumeric digits.
22
8CWE-307AuthenticationHighSmartstore.Web/Controllers/IdentityController.cs138The application does not implement measures to prevent multiple failed authentication attempts within a short time frame. This can allow any user to make authentication attempts without any set limit. As there is no restriction in place to limit the number of login attempts, a user can directly perform a brute force attack on the application. The lockout on failure option should be turned to true and the limit should be 5 failed login attempts and then a 24 hours cooldown period.
23
9CWE-308AuthenticationInfoSmartstore.Web/Controllers/IdentityController.cs112-170The application uses a Single Factor of authentication that too with a weak password complexity policy. The majority of the users would use commonly reused and easy-to-crack passwords. Relying on a single factor can act as a risk in the future for Smart store.Because of the weak password policy, no account failure, and a single factor of authentication a single mistake from a user can easily expose their password and its hash.Use of multi-factor authentication with an MFA Code provided with either SMS, Email, or Authenticator Application.
24
10CWE-338CryptographyMediumSmartstore.Core/Platform/Security/Settings/SecuritySettings.cs16The developer is using a not cryptographically strong Random() function to generate the encryption key. This function is not designed for cryptographic context and can expose cryptography in certain attacks. If a cryptographically weak random number generator is used as a seed for the cryptographic key then the attacker can easily guess the ID of the key. Instead of using Random() using System.Security.Cryptography.RandomNumberGenerator() function which is cryptographically strong.
25
11CWE-89Input HandlingInfoSmartstore.Data/Smartstore.Data.MySql/MySqlDataProvider.cs; Smartstore.Data/Smartstore.Data.SqlServer/SqlServerDataProvider.cs; Smartstore/Data/Providers/DataProvider.cs; 91, 97, 115, 126; 184, 191; 311, 320;The developer is executing Raw SQL queries in all of these files and methods. Such SQL Queries are difficult to maintain and pose a higher risk of SQL Injection attacks. If an attacker can manipulate the query they can perform destructive actions on the application. Majority of the Data Provider SQL functions using Raw SQL queries are not taking input from the user which should reduce the possibility of a SQL Injection attack. Although, some of the methods are truncating tables and can be exploited given the right conditions. Instead of using RAW SQL queries the developer can use LINQ queries to interact with the database. LINQ passes all the data to SQL via SQL parameters and it is composed dynamically which considerably reduces the chances of an SQL Injection attack.
26
12CWE-614Session ManagementMediumSmartstore.Core/Catalog/Products/Services/ProductCompareService.cs112-116In the SetComparedProductIds method of the ProductCompareService, when creating a cookie of the ComparedProducts the developer has not configured the cookie with the secure flag set as true. The secure flag is necessary to ensure that the cookie is only sent over an encrypted HTTPS session and not an HTTP session. Cookies over HTTP are sent in clear text and can be read by anyone sniffing the traffic. If the attacker can eaves drop on the network, they would be able to see all the contents of the ComparedProducts cookie from the SmartStore application in cleartext. The secure flag should be set on all cookies that are transmitting sensitive/user data. This will ensure cookies are only transmitted over an HTTPS session and not on an unencrypted HTTP session.
27
13CWE-614Session ManagementMediumSmartstore.Core/Catalog/Products/Services/RecentlyViewedProductsService.cs72-77In the AddProductToRecentlyViewedList method of the RecentlyViewedProductsService, when creating a cookie of the RecentlyViewedProducts the developer has not configured the cookie with the secure flag set as true. The secure flag is necessary to ensure that the cookie is only sent over an encrypted HTTPS session and not an HTTP session. Cookies over HTTP are sent in clear text and can be read by anyone sniffing the traffic. If the attacker can eaves drop on the network, they would be able to see all the contents of the RecentlyViewedProducts cookie from the SmartStore application in cleartext. The secure flag should be set on all cookies that are transmitting sensitive/user data. This will ensure cookies are only transmitted over an HTTPS session and not on an unencrypted HTTP session.
28
14CWE-614Session ManagementMediumSmartstore.Core/Platform/Installation/InstallationService.cs542-548In the SaveCurrentLanguage method of the InstallationService, when creating a cookie of the LanguageCookieName the developer has not configured the cookie with the secure flag set as true. The secure flag is necessary to ensure that the cookie is only sent over an encrypted HTTPS session and not an HTTP session. Cookies over HTTP are sent in clear text and can be read by anyone sniffing the traffic. If the attacker can eaves drop on the network, they would be able to see all the contents of the LanguageCookieName cookie from the SmartStore application in cleartext. The secure flag should be set on all cookies that are transmitting sensitive/user data. This will ensure cookies are only transmitted over an HTTPS session and not on an unencrypted HTTP session.
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100