Skip to content
This repository has been archived by the owner on May 10, 2024. It is now read-only.

Commit

Permalink
Fix #6277 Detect unknown unicode in sign message requests and some im…
Browse files Browse the repository at this point in the history
…provement for misleading message (#6300)
  • Loading branch information
nuo-xu authored Nov 2, 2022
1 parent df087be commit a2991d7
Show file tree
Hide file tree
Showing 3 changed files with 206 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,17 @@ struct SignatureRequestView: View {
var onDismiss: () -> Void

@State private var requestIndex: Int = 0
/// A map between request index and a boolean value indicates this request message needs pilcrow formating
@State private var needPilcrowFormatted: [Int: Bool] = [0: false]
/// A map between request index and a boolean value indicates this request message is displayed as
/// its original content
@State private var showOrignalMessage: [Int: Bool] = [0: true]
@Environment(\.sizeCategory) private var sizeCategory
@Environment(\.presentationMode) @Binding private var presentationMode
@Environment(\.pixelLength) private var pixelLength
@ScaledMetric private var blockieSize = 54
private let maxBlockieSize: CGFloat = 108
private let staticTextViewHeight: CGFloat = 200

private var currentRequest: BraveWallet.SignMessageRequest {
requests[requestIndex]
Expand All @@ -30,6 +37,30 @@ struct SignatureRequestView: View {
keyringStore.allAccounts.first(where: { $0.address == currentRequest.address }) ?? keyringStore.selectedAccount
}

private var requestMessage: String {
if showOrignalMessage[requestIndex] == true {
return currentRequest.message
} else {
let uuid = UUID()
var result = currentRequest.message
if needPilcrowFormatted[requestIndex] == true {
var copy = currentRequest.message
while copy.range(of: "\\n{2,}", options: .regularExpression) != nil {
if let range = copy.range(of: "\\n{2,}", options: .regularExpression) {
let newlines = String(copy[range])
result.replaceSubrange(range, with: "\n\(uuid.uuidString) <\(newlines.count)>\n")
copy.replaceSubrange(range, with: "\n\(uuid.uuidString) <\(newlines.count)>\n")
}
}
}
if currentRequest.message.hasUnknownUnicode {
result = result.printableWithUnknownUnicode
}

return result.replacingOccurrences(of: uuid.uuidString, with: "\u{00B6}")
}
}

init(
requests: [BraveWallet.SignMessageRequest],
keyringStore: KeyringStore,
Expand Down Expand Up @@ -81,18 +112,52 @@ struct SignatureRequestView: View {
Text(Strings.Wallet.signatureRequestSubtitle)
.font(.headline)
.foregroundColor(Color(.bravePrimary))
if needPilcrowFormatted[requestIndex] == true || currentRequest.message.hasUnknownUnicode == true {
VStack(spacing: 8) {
if needPilcrowFormatted[requestIndex] == true {
Text("\(Image(systemName: "exclamationmark.triangle.fill")) \(Strings.Wallet.signMessageConsecutiveNewlineWarning)")
.font(.subheadline.weight(.medium))
.foregroundColor(Color(.braveLabel))
.multilineTextAlignment(.center)
}
if currentRequest.message.hasUnknownUnicode == true {
Text("\(Image(systemName: "exclamationmark.triangle.fill")) \(Strings.Wallet.signMessageRequestUnknownUnicodeWarning)")
.font(.subheadline.weight(.medium))
.foregroundColor(Color(.braveLabel))
.multilineTextAlignment(.center)
}
Button {
let value = showOrignalMessage[requestIndex] ?? false
showOrignalMessage[requestIndex] = !value
} label: {
Text(showOrignalMessage[requestIndex] == true ? Strings.Wallet.signMessageShowUnknownUnicode : Strings.Wallet.signMessageShowOriginalMessage)
.font(.subheadline)
.foregroundColor(Color(.braveBlurple))
}
}
.padding(12)
.frame(maxWidth: .infinity)
.background(
Color(.braveWarningBackground)
.overlay(
RoundedRectangle(cornerRadius: 10, style: .continuous)
.strokeBorder(Color(.braveWarningBorder), style: StrokeStyle(lineWidth: pixelLength))
)
.clipShape(RoundedRectangle(cornerRadius: 10, style: .continuous))
)
}
}
.padding(.vertical, 32)
StaticTextView(text: currentRequest.message, isMonospaced: false)
StaticTextView(text: requestMessage, isMonospaced: false)
.frame(maxWidth: .infinity)
.frame(height: 200)
.frame(height: staticTextViewHeight)
.background(Color(.tertiaryBraveGroupedBackground))
.clipShape(RoundedRectangle(cornerRadius: 5, style: .continuous))
.padding()
.background(
Color(.secondaryBraveGroupedBackground)
)
.clipShape(RoundedRectangle(cornerRadius: 10, style: .continuous))
.background(
Color(.secondaryBraveGroupedBackground)
)
.clipShape(RoundedRectangle(cornerRadius: 10, style: .continuous))
buttonsContainer
.padding(.top)
.opacity(sizeCategory.isAccessibilityCategory ? 0 : 1)
Expand Down Expand Up @@ -128,6 +193,17 @@ struct SignatureRequestView: View {
.navigationBarTitleDisplayMode(.inline)
.foregroundColor(Color(.braveLabel))
.background(Color(.braveGroupedBackground).edgesIgnoringSafeArea(.all))
.introspectTextView { textView in
// A flash to show users message is overflowing the text view (related to issue https://github.com/brave/brave-ios/issues/6277)
if showOrignalMessage[requestIndex] == true {
if textView.contentSize.height > staticTextViewHeight && currentRequest.message.hasConsecutiveNewLines {
needPilcrowFormatted[requestIndex] = true
textView.flashScrollIndicators()
} else {
needPilcrowFormatted[requestIndex] = false
}
}
}
}

private var isButtonsDisabled: Bool {
Expand All @@ -149,6 +225,7 @@ struct SignatureRequestView: View {
@ViewBuilder private var buttons: some View {
Button(action: { // cancel
cryptoStore.handleWebpageRequestResponse(.signMessage(approved: false, id: currentRequest.id))
updateState()
if requests.count == 1 {
onDismiss()
}
Expand All @@ -160,6 +237,7 @@ struct SignatureRequestView: View {
.disabled(isButtonsDisabled)
Button(action: { // approve
cryptoStore.handleWebpageRequestResponse(.signMessage(approved: true, id: currentRequest.id))
updateState()
if requests.count == 1 {
onDismiss()
}
Expand All @@ -171,15 +249,72 @@ struct SignatureRequestView: View {
.disabled(isButtonsDisabled)
}

private func updateState() {
var newShowOrignalMessage: [Int: Bool] = [:]
showOrignalMessage.forEach { key, value in
if key != 0 {
newShowOrignalMessage[key - 1] = value
}
}
showOrignalMessage = newShowOrignalMessage

var newNeedPilcrowFormatted: [Int: Bool] = [:]
needPilcrowFormatted.forEach { key, value in
if key != 0 {
newNeedPilcrowFormatted[key - 1] = value
}
}
needPilcrowFormatted = newNeedPilcrowFormatted
}

private func next() {
if requestIndex + 1 < requests.count {
requestIndex += 1
let value = requestIndex + 1
if showOrignalMessage[value] == nil {
showOrignalMessage[value] = true
}
requestIndex = value
} else {
requestIndex = 0
}
}
}

extension String {
var hasUnknownUnicode: Bool {
// same requirement as desktop. Valid: [0, 127]
for c in unicodeScalars {
let ci = Int(c.value)
if ci > 127 {
return true
}
}
return false
}

var hasConsecutiveNewLines: Bool {
// return true if string has two or more consecutive newline chars
return range(of: "\\n{2,}", options: .regularExpression) != nil
}

var printableWithUnknownUnicode: String {
var result = ""
for c in unicodeScalars {
let ci = Int(c.value)
if let unicodeScalar = Unicode.Scalar(ci) {
if ci == 10 { // will keep newline char as it is
result += "\n"
} else {
// ascii char will be displayed as it is
// unknown (> 127) will be displayed as hex-encoded
result += unicodeScalar.escaped(asASCII: true)
}
}
}
return result
}
}

#if DEBUG
struct SignatureRequestView_Previews: PreviewProvider {
static var previews: some View {
Expand Down
28 changes: 28 additions & 0 deletions Sources/BraveWallet/WalletStrings.swift
Original file line number Diff line number Diff line change
Expand Up @@ -3221,5 +3221,33 @@ extension Strings {
value: "Details",
comment: "The title on top of a separater, and the transaction details will be displayed below the separater."
)
public static let signMessageRequestUnknownUnicodeWarning = NSLocalizedString(
"wallet.signMessageRequestUnknownUnicodeWarning",
tableName: "BraveWallet",
bundle: .module,
value: "Non-ASCII characters detected!",
comment: "A warning message to tell users that the sign request message contains non-ascii characters."
)
public static let signMessageConsecutiveNewlineWarning = NSLocalizedString(
"wallet.signMessageRequestUnknownUnicodeWarning",
tableName: "BraveWallet",
bundle: .module,
value: "Consecutive newline characters detected!",
comment: "A warning message to tell users that the sign request message contains consecutive new-line characters."
)
public static let signMessageShowUnknownUnicode = NSLocalizedString(
"wallet.signMessageRequestUnknownUnicodeWarning",
tableName: "BraveWallet",
bundle: .module,
value: "Show encoded message",
comment: "The title of the button that users can click to display the sign request message in ASCII encoding."
)
public static let signMessageShowOriginalMessage = NSLocalizedString(
"wallet.signMessageRequestUnknownUnicodeWarning",
tableName: "BraveWallet",
bundle: .module,
value: "View original message",
comment: "The title of the button that users can click to display the sign request message as its original content."
)
}
}
36 changes: 36 additions & 0 deletions Tests/BraveWalletTests/WalletStringExtensionTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// Copyright 2022 The Brave Authors. All rights reserved.
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this
// file, You can obtain one at http://mozilla.org/MPL/2.0/.

import XCTest
@testable import BraveWallet

import Foundation

class WalletStringExtensionTests: XCTestCase {
func testHasUnknownUnicode() {
let stringHasOnlyKnownUnicode = "hello this is message is in ascii chars only"
XCTAssertFalse(stringHasOnlyKnownUnicode.hasUnknownUnicode)

let stringsHasUnknownUnicode = "\u{202E} EVIL"
XCTAssertTrue(stringsHasUnknownUnicode.hasUnknownUnicode)
}

func testHasConsecutiveNewlines() {
let stringHasNoNewlines = "Here is one sentence."
XCTAssertFalse(stringHasNoNewlines.hasConsecutiveNewLines)

let stringsHasNewlineButNoConsecutiveNewlines = "Here is one sentence.\nAnd here is another."
XCTAssertFalse(stringsHasNewlineButNoConsecutiveNewlines.hasConsecutiveNewLines)

let stringHasConsecutiveNewlines = "Main Message\n\n\n\n\nEvil payload is below"
XCTAssertTrue(stringHasConsecutiveNewlines.hasConsecutiveNewLines)
}

func testPrintableWithUnknownUnicode() {
let string = "Main Message\nEvil payload is below \n\npaylod still loooks good\nNew Line\n\n\u{202E} EVIL"
let printable = "Main Message\nEvil payload is below \n\\u{00B6}\npaylod still loooks good\nNew Line\n\\u{00B6}\n\\u{202E} EVIL"
XCTAssertEqual(string.printableWithUnknownUnicode, printable)
}
}

0 comments on commit a2991d7

Please sign in to comment.