https://github.com/DexlynLabs/bridge-repo/tree/6a772c126e0879928021156ca91b89ac4f1bfcae/move
The Name
and Symbol
variables in the constructor of contract HypERC721.sol are not filtered in any way, allowing an attacker to create a token with malicious JavaScript or HTML code injected into these fields. These fields are also not limited by character count, enabling the injection of a large amount of code. If no mitigation mechanisms are implemented in the web application for malicious code from these variables, the application will be vulnerable to XSS or HTML injection attacks if the values of these variables are displayed in the web application.
https://github.com/DexlynLabs/bridge-repo/blob/6a772c126e0879928021156ca91b89ac4f1bfcae/solidity/contracts/token/HypERC721.sol
If an attacker creates an asset with a symbol containing the malicious javascript payload, he could get a stored XSS on this website that render his malicious NFT name and symbol, which is legitimately generated by this dapp, according to correspondence with the sponsor, there is a possibility of transferring the created NFT to other applications like NFT exchanges, which creates an additional XSS risk on the mentioned dApps.. This could allow the attacker for example, to run a keylogger script to collect all inputs typed by a user including his password or to create a fake Metamask pop up asking a user to sign a malicious transaction.
https://solodit.cyfrin.io/issues/insufficient-input-validation-on-sablierv2nftdescriptorsafeassetsymbol-allows-an-attacker-to-obtain-stored-xss-codehawks-sablier-git
https://osec.io/blog/2023-08-11-web2-bug-repellant-instructions
In cybersecurity, principles such as Defense in Depth emphasize that mitigation mechanisms should be applied at every layer of the system, application, or smart contract. Just as mitigation mechanisms against code injection exist for backend languages like Python, similar measures should be implemented in Solidity, in addition to frontend protections.
https://en.wikipedia.org/wiki/Defense_in_depth_(computing)
it's absolutely necessary to sanitize the user's input in the constructor. The asset symbol should only contain Aa-Zz
and 0-9
characters while forbidding special ones, i.e. < / >
. The length of possible characters should also be significantly limited. The principle of security in depth should be applied, securing each possible injection point in the best possible way.
require(bytes(symbol_).length <= 7, "NFT: symbol too long");
bytes memory symbolBytes = bytes(symbol_);
for(uint i = 0; i < symbolBytes.length; i++) {
bytes1 char = symbolBytes[i];
require(
char != bytes1('<') &&
char != bytes1('>') &&
char != bytes1('{') &&
char != bytes1('}'),
"NFT : invalid symbol character"
);
}``
I know that this severity is not in scope, but maybe it will bring value to your client.
By using the written test file, I was able to modify this line of code https://github.com/DexlynLabs/bridge-repo/blob/6a772c126e0879928021156ca91b89ac4f1bfcae/solidity/test/token/HypERC721.t.sol#L37 in the following way, and the test was successful.
string internal constant SYMBOL = "<script>alert('1')</script><script>alert('1')</script><script>alert('1')</script><script>alert('1')</script><script>alert('1')</script><script>alert('1')</script><script>alert('1')</script><script>alert('1')</script><script>alert('1')</script><script>alert('1')</script><script>alert('1')</script><script>alert('1')</script><script>alert('1')</script><script>alert('1')</script><script>alert('1')</script><script>alert('1')</script><script>alert('1')</script><script>alert('1')</script><script>alert('1')</script>", "<script>alert('1')</script><script>alert('1')</script><script>alert('1')</script><script>alert('1')</script><script>alert('1')</script><script>alert('1')</script><script>alert('1')</script><script>alert('1')</script><script>alert('1')</script><script>alert('1')</script><script>alert('1')</script><script>alert('1')</script><script>alert('1')</script><script>alert('1')</script><script>alert('1')</script><script>alert('1')</script><script>alert('1')</script><script>alert('1')</script><script>alert('1')</script><script>alert('1')</script>";