Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: Replace md5 with sha256 in createFastHash(). #31748

Open
cryptochecktool opened this issue Dec 13, 2024 · 12 comments
Open

Bug: Replace md5 with sha256 in createFastHash(). #31748

cryptochecktool opened this issue Dec 13, 2024 · 12 comments
Labels
Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug

Comments

@cryptochecktool
Copy link

cryptochecktool commented Dec 13, 2024

1.MD5 has potential collision risks.
2.Even for speed considerations, MD5 is slower than SHA256, so replacing MD5 with SHA256 is worthwhile.
code at:

export function createFastHash(input: string): string | number {
const hash = createHash('md5');
hash.update(input);
return hash.digest('hex');
}

Below are the speed test results(:

String Length: 16

MD5 Average Time: 0.0148 ms
SHA256 Average Time: 0.0131 ms

String Length: 256

MD5 Average Time: 0.0193 ms
SHA256 Average Time: 0.0175 ms

String Length: 10000

MD5 Average Time: 0.5830 ms
SHA256 Average Time: 0.4030 ms

String Length: 100000

MD5 Average Time: 6.2866 ms
SHA256 Average Time: 4.6130 ms

code is

<!DOCTYPE html>
<html>
<head>
    <title>Hash Function Test</title>
    <!-- Include the CryptoJS library -->
    <script src="https://cdnjs.cloudflare.com/ajax/libs/crypto-js/4.1.1/crypto-js.min.js"></script>
</head>
<body>
    <h1>Hash Function Performance Test</h1>
    <div id="results"></div>

    <script>
        const md5 = (str) => CryptoJS.MD5(str).toString();
        const sha256 = (str) => CryptoJS.SHA256(str).toString();

        // Generate a random string of specified length
        function getRandomString(length) {
            let result = '';
            const characters = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789';
            for (let i = 0; i < length; i++) {
                result += characters.charAt(Math.floor(Math.random() * characters.length));
            }
            return result;
        }

        // Test the performance of the hash function
        function testHashFunction(func, length, iterations = 1000) {
            let total = 0;
            const data = getRandomString(length); // Generate data once outside the loop
            const startTime = performance.now();

            for (let i = 0; i < iterations; i++) {
                func(data); // Reuse the same data for all iterations
            }

            const endTime = performance.now();
            total = endTime - startTime; // Calculate total time once
            const average = total / iterations;
            return average;
        }

        // Test different string lengths
        const lengths = [16, 256, 10000, 100000];
        const resultsElement = document.getElementById('results');

        lengths.forEach(length => {
            const md5Avg = testHashFunction(md5, length);
            const sha256Avg = testHashFunction(sha256, length);

            resultsElement.innerHTML += `
                <h2>String Length: ${length}</h2>
                <p>MD5 Average Time: ${md5Avg.toFixed(4)} ms</p>
                <p>SHA256 Average Time: ${sha256Avg.toFixed(4)} ms</p>
                <hr>
            `;
        });
    </script>
</body>
</html>
@cryptochecktool cryptochecktool added the Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug label Dec 13, 2024
@Gatormane35
Copy link

1.MD5 has potential collision risks. 2.Even for speed considerations, MD5 is slower than SHA256, so replacing MD5 with SHA256 is worthwhile. Below are the speed test results(:

String Length: 16

MD5 Average Time: 0.0148 ms SHA256 Average Time: 0.0131 ms

String Length: 256

MD5 Average Time: 0.0193 ms SHA256 Average Time: 0.0175 ms

String Length: 10000

MD5 Average Time: 0.5830 ms SHA256 Average Time: 0.4030 ms

String Length: 100000

MD5 Average Time: 6.2866 ms SHA256 Average Time: 4.6130 ms

code is

<!DOCTYPE html>
<html>
<head>
    <title>Hash Function Test</title>
    <!-- Include the CryptoJS library -->
    <script src="https://cdnjs.cloudflare.com/ajax/libs/crypto-js/4.1.1/crypto-js.min.js"></script>
</head>
<body>
    <h1>Hash Function Performance Test</h1>
    <div id="results"></div>

    <script>
        const md5 = (str) => CryptoJS.MD5(str).toString();
        const sha256 = (str) => CryptoJS.SHA256(str).toString();

        // Generate a random string of specified length
        function getRandomString(length) {
            let result = '';
            const characters = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789';
            for (let i = 0; i < length; i++) {
                result += characters.charAt(Math.floor(Math.random() * characters.length));
            }
            return result;
        }

        // Test the performance of the hash function
        function testHashFunction(func, length, iterations = 1000) {
            let total = 0;
            const data = getRandomString(length); // Generate data once outside the loop
            const startTime = performance.now();

            for (let i = 0; i < iterations; i++) {
                func(data); // Reuse the same data for all iterations
            }

            const endTime = performance.now();
            total = endTime - startTime; // Calculate total time once
            const average = total / iterations;
            return average;
        }

        // Test different string lengths
        const lengths = [16, 256, 10000, 100000];
        const resultsElement = document.getElementById('results');

        lengths.forEach(length => {
            const md5Avg = testHashFunction(md5, length);
            const sha256Avg = testHashFunction(sha256, length);

            resultsElement.innerHTML += `
                <h2>String Length: ${length}</h2>
                <p>MD5 Average Time: ${md5Avg.toFixed(4)} ms</p>
                <p>SHA256 Average Time: ${sha256Avg.toFixed(4)} ms</p>
                <hr>
            `;
        });
    </script>
</body>
</html>

@Gatormane35
Copy link

Gatormane35 commented Dec 13, 2024

Kwamanabrown1@Gateway229

@Gatormane35
Copy link

[email protected]

@cryptochecktool
Copy link
Author

@Gateway229 what's your meaning?

@Gatormane35
Copy link

1.MD5 has potential collision risks. 2.Even for speed considerations, MD5 is slower than SHA256, so replacing MD5 with SHA256 is worthwhile. Below are the speed test results(:

String Length: 16

MD5 Average Time: 0.0148 ms SHA256 Average Time: 0.0131 ms

String Length: 256

MD5 Average Time: 0.0193 ms SHA256 Average Time: 0.0175 ms

String Length: 10000

MD5 Average Time: 0.5830 ms SHA256 Average Time: 0.4030 ms

String Length: 100000

MD5 Average Time: 6.2866 ms SHA256 Average Time: 4.6130 ms

code is

<!DOCTYPE html>
<html>
<head>
    <title>Hash Function Test</title>
    <!-- Include the CryptoJS library -->
    <script src="https://cdnjs.cloudflare.com/ajax/libs/crypto-js/4.1.1/crypto-js.min.js"></script>
</head>
<body>
    <h1>Hash Function Performance Test</h1>
    <div id="results"></div>

    <script>
        const md5 = (str) => CryptoJS.MD5(str).toString();
        const sha256 = (str) => CryptoJS.SHA256(str).toString();

        // Generate a random string of specified length
        function getRandomString(length) {
            let result = '';
            const characters = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789';
            for (let i = 0; i < length; i++) {
                result += characters.charAt(Math.floor(Math.random() * characters.length));
            }
            return result;
        }

        // Test the performance of the hash function
        function testHashFunction(func, length, iterations = 1000) {
            let total = 0;
            const data = getRandomString(length); // Generate data once outside the loop
            const startTime = performance.now();

            for (let i = 0; i < iterations; i++) {
                func(data); // Reuse the same data for all iterations
            }

            const endTime = performance.now();
            total = endTime - startTime; // Calculate total time once
            const average = total / iterations;
            return average;
        }

        // Test different string lengths
        const lengths = [16, 256, 10000, 100000];
        const resultsElement = document.getElementById('results');

        lengths.forEach(length => {
            const md5Avg = testHashFunction(md5, length);
            const sha256Avg = testHashFunction(sha256, length);

            resultsElement.innerHTML += `
                <h2>String Length: ${length}</h2>
                <p>MD5 Average Time: ${md5Avg.toFixed(4)} ms</p>
                <p>SHA256 Average Time: ${sha256Avg.toFixed(4)} ms</p>
                <hr>
            `;
        });
    </script>
</body>
</html>

@narendrayogi
Copy link

Any connection with react project?🤔

@cryptochecktool
Copy link
Author

Any connection with react project?🤔

code at:

const hash = createHash('md5');

@narendrayogi
Copy link

Guess that varies depending on the requirement and system performance!

Screenshot 2024-12-19 at 21 19 12

My system results. As you stated SHA256 is better when comes to large strings

Screenshot 2024-12-19 at 21 24 05

@cryptochecktool
Copy link
Author

Since 2013, intel has supported instruction set acceleration for sha256. (AMD is following suit.) So SHA256 is faster than MD5.

@cryptochecktool
Copy link
Author

I think someone should fix this problem. I think it will be fixed quickly. But I have no experience with js development, and if you could submit a PR to fix it, that would have a positive impact on React speed.

@floweeb
Copy link

floweeb commented Dec 22, 2024

Is anyone doing this I'd like to fix it.

@floweeb
Copy link

floweeb commented Dec 26, 2024

I finished it yesterday @cryptochecktool and submitted a PR.
#31910

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug
Projects
None yet
Development

No branches or pull requests

4 participants