From 87e7978e216d6a8102655e77bda6ba5eb1d9cc03 Mon Sep 17 00:00:00 2001 From: Gary Ewan Park Date: Thu, 22 Feb 2018 10:02:39 +0000 Subject: [PATCH] (GH-1505) Ensure package information is escaped - Also, when de-serializing, if a .backup file exists, remove original - Warn user that .backup file exists and needs to be corrected --- .../domain/RegistryValueExtensions.cs | 6 +++- .../ChocolateyPackageInformationService.cs | 13 +++++++- .../infrastructure/services/XmlService.cs | 32 +++++++------------ 3 files changed, 28 insertions(+), 23 deletions(-) diff --git a/src/chocolatey/infrastructure.app/domain/RegistryValueExtensions.cs b/src/chocolatey/infrastructure.app/domain/RegistryValueExtensions.cs index b754b7f3a9..118981b85e 100644 --- a/src/chocolatey/infrastructure.app/domain/RegistryValueExtensions.cs +++ b/src/chocolatey/infrastructure.app/domain/RegistryValueExtensions.cs @@ -24,7 +24,11 @@ public static string get_value_as_string(this RegistryKey key, string name) { if (key == null) return string.Empty; - return key.GetValue(name).to_string().Replace("\0", string.Empty); + // Since it is possible that registry keys contain characters that are not valid + // in XML files, ensure that all content is escaped, prior to serialization + var escapedXml = System.Security.SecurityElement.Escape(key.GetValue(name).to_string()); + + return escapedXml?.Replace("\0", string.Empty); } } } diff --git a/src/chocolatey/infrastructure.app/services/ChocolateyPackageInformationService.cs b/src/chocolatey/infrastructure.app/services/ChocolateyPackageInformationService.cs index ce94753d58..f7eb12260a 100644 --- a/src/chocolatey/infrastructure.app/services/ChocolateyPackageInformationService.cs +++ b/src/chocolatey/infrastructure.app/services/ChocolateyPackageInformationService.cs @@ -29,6 +29,7 @@ public class ChocolateyPackageInformationService : IChocolateyPackageInformation private readonly IRegistryService _registryService; private readonly IFilesService _filesService; private const string REGISTRY_SNAPSHOT_FILE = ".registry"; + private const string REGISTRY_SNAPSHOT_BACKUP_FILE = ".registry.backup"; private const string FILES_SNAPSHOT_FILE = ".files"; private const string SILENT_UNINSTALLER_FILE = ".silentUninstaller"; private const string SIDE_BY_SIDE_FILE = ".sxs"; @@ -62,7 +63,17 @@ public ChocolateyPackageInformation get_package_information(IPackage package) FaultTolerance.try_catch_with_logging_exception( () => { - packageInformation.RegistrySnapshot = _registryService.read_from_file(_fileSystem.combine_paths(pkgStorePath, REGISTRY_SNAPSHOT_FILE)); + if (_fileSystem.file_exists(_fileSystem.combine_paths(pkgStorePath, REGISTRY_SNAPSHOT_BACKUP_FILE))) + { + // Remove original file, since it was corrupt + _fileSystem.delete_file(_fileSystem.combine_paths(pkgStorePath, REGISTRY_SNAPSHOT_FILE)); + + this.Log().Warn("A corrupt .registry file exists at {0}, please correct the file and rename.", _fileSystem.combine_paths(pkgStorePath, REGISTRY_SNAPSHOT_FILE)); + } + else + { + packageInformation.RegistrySnapshot = _registryService.read_from_file(_fileSystem.combine_paths(pkgStorePath, REGISTRY_SNAPSHOT_FILE)); + } }, "Unable to read registry snapshot file for {0} (located at {1})".format_with(package.Id, _fileSystem.combine_paths(pkgStorePath, REGISTRY_SNAPSHOT_FILE)), throwError: false, diff --git a/src/chocolatey/infrastructure/services/XmlService.cs b/src/chocolatey/infrastructure/services/XmlService.cs index 3a7236b5f7..50f45141a6 100644 --- a/src/chocolatey/infrastructure/services/XmlService.cs +++ b/src/chocolatey/infrastructure/services/XmlService.cs @@ -45,7 +45,7 @@ public XmlService(IFileSystem fileSystem, IHashProvider hashProvider) public XmlType deserialize(string xmlFilePath) { - return FaultTolerance.retry(3, () => GlobalMutex.enter( + return FaultTolerance.retry(3, () => GlobalMutex.enter( () => { this.Log().Trace("Entered mutex to deserialize '{0}'".format_with(xmlFilePath)); @@ -73,27 +73,17 @@ public XmlType deserialize(string xmlFilePath) // Check if its just a malformed document. if (ex.Message.Contains("There is an error in XML document")) { - // If so, check for a backup file and try an parse that. - if (_fileSystem.file_exists(xmlFilePath + ".backup")) - { - using (var backupStream = _fileSystem.open_file_readonly(xmlFilePath + ".backup")) - using (var backupReader = new StreamReader(backupStream)) - using (var backupXmlReader = XmlReader.Create(backupReader)) - { - var validConfig = (XmlType)xmlSerializer.Deserialize(backupXmlReader); - - // If there's no errors and it's valid, go ahead and replace the bad file with the backup. - if (validConfig != null) - { - _fileSystem.copy_file(xmlFilePath + ".backup", xmlFilePath, overwriteExisting: true); - } - - return validConfig; - } - } - } + // Move file to a backup location, so that it isn't parsed again + _fileSystem.copy_file(xmlFilePath, xmlFilePath + ".backup", overwriteExisting: true); - throw; + this.Log().Warn("Unable to deserialize XML located at {0}", xmlFilePath); + this.Log().Warn("This file has been moved to {0}. Please correct the file, and then rename the file, removing .backup", xmlFilePath + ".backup"); + return default(XmlType); + } + else + { + throw; + } } finally {