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

Fix #6277 Detect unknown unicode in sign message requests and some improvement for misleading message #6300

Merged
merged 4 commits into from
Nov 2, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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)
}
}