From b9ef596bfb23055fb07171c16254509c69dacb8e Mon Sep 17 00:00:00 2001 From: Yishi Lin Date: Sun, 15 Oct 2017 21:37:00 +0800 Subject: [PATCH] Add more password name checks. --- .../AddPasswordTableViewController.swift | 9 +--- .../EditPasswordTableViewController.swift | 12 ++---- .../PasswordEditorTableViewController.swift | 41 +++++++++++++++++++ passKit/Helpers/AppError.swift | 3 ++ passKit/Models/PasswordStore.swift | 7 ++++ 5 files changed, 56 insertions(+), 16 deletions(-) diff --git a/pass/Controllers/AddPasswordTableViewController.swift b/pass/Controllers/AddPasswordTableViewController.swift index c536f80..e69cfcf 100644 --- a/pass/Controllers/AddPasswordTableViewController.swift +++ b/pass/Controllers/AddPasswordTableViewController.swift @@ -39,10 +39,7 @@ class AddPasswordTableViewController: PasswordEditorTableViewController { } // check name - guard nameCell?.getContent()?.isEmpty == false else { - let alertTitle = "Cannot Add Password" - let alertMessage = "Please fill in the name." - Utils.alert(title: alertTitle, message: alertMessage, controller: self, completion: nil) + guard checkName() == true else { return false } } @@ -57,9 +54,7 @@ class AddPasswordTableViewController: PasswordEditorTableViewController { plainText.append("\n") plainText.append(additionsString) } - let encodedName = (nameCell?.getContent()?.stringByAddingPercentEncodingForRFC3986())! - let name = URL(string: encodedName)!.lastPathComponent - let url = URL(string: encodedName)!.appendingPathExtension("gpg") + let (name, url) = getNameURL() password = Password(name: name, url: url, plainText: plainText) } } diff --git a/pass/Controllers/EditPasswordTableViewController.swift b/pass/Controllers/EditPasswordTableViewController.swift index ade537b..1c9cc82 100644 --- a/pass/Controllers/EditPasswordTableViewController.swift +++ b/pass/Controllers/EditPasswordTableViewController.swift @@ -27,12 +27,8 @@ class EditPasswordTableViewController: PasswordEditorTableViewController { override func shouldPerformSegue(withIdentifier identifier: String, sender: Any?) -> Bool { if identifier == "saveEditPasswordSegue" { - if let name = nameCell?.getContent(), - let path = name.stringByAddingPercentEncodingForRFC3986(), - let _ = URL(string: path) { - return true - } else { - Utils.alert(title: "Cannot Save", message: "Password name is invalid.", controller: self, completion: nil) + // check name + guard checkName() == true else { return false } } @@ -47,9 +43,7 @@ class EditPasswordTableViewController: PasswordEditorTableViewController { plainText.append("\n") plainText.append(additionsString) } - let encodedName = (nameCell?.getContent()?.stringByAddingPercentEncodingForRFC3986())! - let name = URL(string: encodedName)!.lastPathComponent - let url = URL(string: encodedName)!.appendingPathExtension("gpg") + let (name, url) = getNameURL() if password!.plainText != plainText || password!.url!.path != url.path { password!.updatePassword(name: name, url: url, plainText: plainText) } diff --git a/pass/Controllers/PasswordEditorTableViewController.swift b/pass/Controllers/PasswordEditorTableViewController.swift index 63cefe2..e733324 100644 --- a/pass/Controllers/PasswordEditorTableViewController.swift +++ b/pass/Controllers/PasswordEditorTableViewController.swift @@ -261,4 +261,45 @@ class PasswordEditorTableViewController: UITableViewController, FillPasswordTabl tableData[additionsSection][0][PasswordEditorCellKey.content] = additionsCell?.getContent() } } + + func getNameURL() -> (String, URL) { + let encodedName = (nameCell?.getContent()?.stringByAddingPercentEncodingForRFC3986())! + let name = URL(string: encodedName)!.lastPathComponent + let url = URL(string: encodedName)!.appendingPathExtension("gpg") + return (name, url) + } + + func checkName() -> Bool { + // the name field should not be empty + guard let name = nameCell?.getContent(), name.isEmpty == false else { + Utils.alert(title: "Cannot Save", message: "Please fill in the name.", controller: self, completion: nil) + return false + } + + // the name should not start with / + guard name.hasPrefix("/") == false else { + Utils.alert(title: "Cannot Save", message: "Please remove the prefix \"/\" from your password name.", controller: self, completion: nil) + return false + } + + // the name field should be a valid url + guard let path = name.stringByAddingPercentEncodingForRFC3986(), + var passwordURL = URL(string: path) else { + Utils.alert(title: "Cannot Save", message: "Password name is invalid.", controller: self, completion: nil) + return false + } + + // check whether we can parse the filename (be consistent with PasswordStore::addPasswordEntities) + var previousURLLength = Int.max + while passwordURL.path != "." { + passwordURL = passwordURL.deletingLastPathComponent() + if passwordURL.absoluteString.count >= previousURLLength { + Utils.alert(title: "Cannot Save", message: "Cannot parse the filename. Please check and simplify the password name.", controller: self, completion: nil) + return false + } + previousURLLength = passwordURL.absoluteString.count + } + + return true + } } diff --git a/passKit/Helpers/AppError.swift b/passKit/Helpers/AppError.swift index 6190c66..5386ee1 100644 --- a/passKit/Helpers/AppError.swift +++ b/passKit/Helpers/AppError.swift @@ -15,6 +15,7 @@ public enum AppError: Error { case PasswordDuplicatedError case GitResetError case PGPPublicKeyNotExistError + case WrongPasswordFilename case UnknownError } @@ -33,6 +34,8 @@ extension AppError: LocalizedError { return "Cannot identify the latest synced commit." case .PGPPublicKeyNotExistError: return "PGP public key doesn't exist." + case .WrongPasswordFilename: + return "Cannot write to the password file." case .UnknownError: return "Unknown error." } diff --git a/passKit/Models/PasswordStore.swift b/passKit/Models/PasswordStore.swift index 0a6f510..45ec596 100644 --- a/passKit/Models/PasswordStore.swift +++ b/passKit/Models/PasswordStore.swift @@ -603,12 +603,19 @@ public class PasswordStore { } var passwordURL = password.url! + var previousURLLength = Int.max var paths: [String] = [] while passwordURL.path != "." { paths.append(passwordURL.path) passwordURL = passwordURL.deletingLastPathComponent() + // better identify errors before saving a new password + if passwordURL.absoluteString.count >= previousURLLength { + throw AppError.WrongPasswordFilename + } + previousURLLength = passwordURL.absoluteString.count } paths.reverse() + print(paths) var parentPasswordEntity: PasswordEntity? = nil for path in paths { let isDir = !path.hasSuffix(".gpg")