VerifyFile accepts modified signed files

[philipp-H]

Versions: PgpCore: 5.3.2 Portable.BouncyCastle : 1.9.0 .Net Framework 4.8

I just started using this library. Maybe I'm doing something wrong. I want to do the following:

• Create a txt file on my system and sign it with my private key
• Send the txt, signed file and my public key to another system
• Verify the signed file with the public key on the other system

I created the following UnitTest to verify my use case. The problem occurs in the method VerifyFile_BadContent. In this method I'm changing the content of the signed file and run the VerifyFile method which still returns true. The same happens with VerifyFile_BadHeader and VerifyFile_BadTrailing.

As far as I can tell, this should not happen. I used cyberchef and gpg on linux to test my findings and both return errors.

To verify, just run the tests in the order listed.

Unit Test:

using Microsoft.VisualStudio.TestTools.UnitTesting;
using PgpCore;
using System;
using System.IO;

namespace PGP
{
    [TestClass]
    public class PgpCore
    {
        [TestMethod]
        public void GenKeys()
        {
            using (PGP pgp = new PGP())
            {
                pgp.GenerateKey(@"C:\TEMP\Keys\public.asc", @"C:\TEMP\Keys\private.asc", "email@email.com", "password");
            }
        }

        [TestMethod]
        public void WriteContentFile()
        {
            var plainLorem = @"Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet. Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et e";

            File.WriteAllText(@"C:\TEMP\Content\content.txt", plainLorem);
        }

        [TestMethod]
        public void SignFile()
        {
            //Load keys
            FileInfo privateKey = new FileInfo(@"C:\TEMP\Keys\private.asc");
            EncryptionKeys encryptionKeys = new EncryptionKeys(privateKey, "password");

            // Reference input/output files
            FileInfo inputFile = new FileInfo(@"C:\TEMP\Content\content.txt");
            FileInfo signedFile = new FileInfo(@"C:\TEMP\Content\signed.pgp");

            // Sign
            PGP pgp = new PGP(encryptionKeys);
            pgp.SignFile(inputFile, signedFile);
        }

        [TestMethod]
        public void VerifyFile()
        {
            // Load keys
            FileInfo publicKey = new FileInfo(@"C:\TEMP\Keys\public.asc");
            EncryptionKeys encryptionKeys = new EncryptionKeys(publicKey);

            // Reference input
            FileInfo inputFile = new FileInfo(@"C:\TEMP\Content\signed.pgp");

            // Verify
            PGP pgp = new PGP(encryptionKeys);
            bool verified = pgp.VerifyFile(inputFile);
            Assert.IsTrue(verified);
        }

        [TestMethod]
        public void VerifyFile_BadContent()
        {
            // Load keys
            FileInfo publicKey = new FileInfo(@"C:\TEMP\Keys\public.asc");
            EncryptionKeys encryptionKeys = new EncryptionKeys(publicKey);

            var data = File.ReadAllLines(@"C:\TEMP\Content\signed.pgp");

            var lastIndex = data[3].Length - 1;
            //var lastChar = data[lastIndex];
            data[3] = data[3].Substring(0, lastIndex - 1) + "x";
            File.WriteAllLines(@"C:\TEMP\Content\signed-with-bad-content.pgp", data);

            // Reference input
            FileInfo inputFile = new FileInfo(@"C:\TEMP\Content\signed-with-bad-content.pgp");

            // Verify
            PGP pgp = new PGP(encryptionKeys);
            bool verified = pgp.VerifyFile(inputFile);
            Assert.IsFalse(verified);
        }

        [TestMethod]
        public void VerifyFile_BadHeader()
        {
            // Load keys
            FileInfo publicKey = new FileInfo(@"C:\TEMP\Keys\public.asc");
            EncryptionKeys encryptionKeys = new EncryptionKeys(publicKey);

            var data = File.ReadAllLines(@"C:\TEMP\Content\signed.pgp");
            data[0] = data[0].Remove(0, 1);
            File.WriteAllLines(@"C:\TEMP\Content\signed-with-bad-header.pgp", data);

            // Reference input
            FileInfo inputFile = new FileInfo(@"C:\TEMP\Content\signed-with-bad-header.pgp");

            // Verify
            PGP pgp = new PGP(encryptionKeys);
            bool verified = pgp.VerifyFile(inputFile);
            Assert.IsFalse(verified);
        }

        [TestMethod]
        public void VerifyFile_BadTrailing()
        {
            // Load keys
            FileInfo publicKey = new FileInfo(@"C:\TEMP\Keys\public.asc");
            EncryptionKeys encryptionKeys = new EncryptionKeys(publicKey);

            var data = File.ReadAllLines(@"C:\TEMP\Content\signed.pgp");
            var lastIndex = data.Length - 1;
            data[lastIndex] = data[lastIndex].Remove(0, 1);
            File.WriteAllLines(@"C:\TEMP\Content\signed-with-bad-trailing.pgp", data);

            // Reference input
            FileInfo inputFile = new FileInfo(@"C:\TEMP\Content\signed-with-bad-trailing.pgp");

            // Verify
            PGP pgp = new PGP(encryptionKeys);
            bool verified = pgp.VerifyFile(inputFile);
            Assert.IsFalse(verified);
        }
    }
}

[mattosaurus]

Hi,

Thanks for this, and for submitting replicable tests :)

I'll have a look into this when I get a chance but it's probably a bug with how I'm verifying files rather than your implementation.

[mattosaurus]

I've just pushed out v5.4.0 which should hopefully fix this.

[philipp-H]

Hello mattosaurus, 

Sorry for the delayed response. I have been busy with unrelated things the past week.  I still have the problem with the corrupted files, even after I updated the NuGet package to 5.4.0.

Maybe I am missing something, but I didn't find any commits/changes regarding the VerifyAsync method https://github.com/mattosaurus/PgpCore/blob/314d47b9d3f610e060b465234b5fa822a7585f90/PgpCore/PGP.cs#L4727.

I created a repo for the unit test that you can checkout: https://github.com/philipp-H/PgpCore.VerifyFile

Best regards.

[mattosaurus]

Looks like I overwote these changes with the other PR. They should be fixed in 5.5 now.