Strip the full set of Unicode bidi controls from attachment filenames.
This commit is contained in:
parent
6aeb145024
commit
c5785c086e
@ -15,6 +15,7 @@ import androidx.core.content.ContextCompat;
|
||||
|
||||
import org.signal.core.ui.CoreUiDependencies;
|
||||
import org.signal.core.ui.R;
|
||||
import org.signal.core.util.BidiUtil;
|
||||
import org.signal.core.util.NoExternalStorageException;
|
||||
import org.signal.core.util.permissions.PermissionCompat;
|
||||
import org.signal.core.ui.permissions.Permissions;
|
||||
@ -179,11 +180,6 @@ public class StorageUtil {
|
||||
}
|
||||
|
||||
public static @Nullable String getCleanFileName(@Nullable String fileName) {
|
||||
if (fileName == null) return null;
|
||||
|
||||
fileName = fileName.replace('\u202D', '\uFFFD');
|
||||
fileName = fileName.replace('\u202E', '\uFFFD');
|
||||
|
||||
return fileName;
|
||||
return BidiUtil.replaceBidiCharacters(fileName);
|
||||
}
|
||||
}
|
||||
|
||||
@ -10,6 +10,9 @@ import java.util.regex.Pattern
|
||||
object BidiUtil {
|
||||
private val ALL_ASCII_PATTERN: Pattern = Pattern.compile("^[\\x00-\\x7F]*$")
|
||||
|
||||
/** The full set of Unicode bidirectional control characters (overrides, isolates, and indicators). */
|
||||
private const val DIRECTIONAL_CHARACTERS = "[\\u200f\\u2066\\u2067\\u2068\\u2069\\u202a\\u202b\\u202c\\u202d\\u202e]"
|
||||
|
||||
object BidiCodepoint {
|
||||
const val LRI = "\u2066"
|
||||
|
||||
@ -160,6 +163,22 @@ object BidiUtil {
|
||||
}
|
||||
|
||||
fun stripAllDirectionalCharacters(text: String): String {
|
||||
return text.replace("[\\u200f\\u2066\\u2067\\u2068\\u2069\\u202a\\u202b\\u202c\\u202d\\u202e]".toRegex(), "")
|
||||
return text.replace(DIRECTIONAL_CHARACTERS.toRegex(), "")
|
||||
}
|
||||
|
||||
/**
|
||||
* Neutralizes the full set of Unicode bidirectional control characters by replacing each one with
|
||||
* the Unicode replacement character (U+FFFD).
|
||||
*
|
||||
* Unlike [stripAllDirectionalCharacters], this preserves a visible indication that a character was
|
||||
* removed. This is useful when sanitizing strings for display where a directional override or
|
||||
* isolate could otherwise be used to spoof content, such as faking an attachment's file extension
|
||||
* (e.g. "document<RLO>txt.exe" rendering as "documentexe.txt").
|
||||
*/
|
||||
@JvmStatic
|
||||
fun replaceBidiCharacters(text: String?): String? {
|
||||
if (text == null) return null
|
||||
|
||||
return text.replace(DIRECTIONAL_CHARACTERS.toRegex(), "<EFBFBD>")
|
||||
}
|
||||
}
|
||||
|
||||
@ -0,0 +1,58 @@
|
||||
package org.signal.core.util
|
||||
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertNull
|
||||
import org.junit.Test
|
||||
|
||||
class BidiUtilTest {
|
||||
|
||||
private val replacement = "<EFBFBD>"
|
||||
|
||||
@Test
|
||||
fun replaceBidiCharacters_nullInput_returnsNull() {
|
||||
assertNull(BidiUtil.replaceBidiCharacters(null))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun replaceBidiCharacters_plainText_isUnchanged() {
|
||||
assertEquals("document.txt", BidiUtil.replaceBidiCharacters("document.txt"))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun replaceBidiCharacters_overrides_areReplaced() {
|
||||
assertEquals("$replacement", BidiUtil.replaceBidiCharacters("")) // LRE
|
||||
assertEquals("$replacement", BidiUtil.replaceBidiCharacters("")) // RLE
|
||||
assertEquals("$replacement", BidiUtil.replaceBidiCharacters("")) // PDF
|
||||
assertEquals("$replacement", BidiUtil.replaceBidiCharacters("")) // LRO
|
||||
assertEquals("$replacement", BidiUtil.replaceBidiCharacters("")) // RLO
|
||||
}
|
||||
|
||||
@Test
|
||||
fun replaceBidiCharacters_isolates_areReplaced() {
|
||||
assertEquals("$replacement", BidiUtil.replaceBidiCharacters("")) // LRI
|
||||
assertEquals("$replacement", BidiUtil.replaceBidiCharacters("")) // RLI
|
||||
assertEquals("$replacement", BidiUtil.replaceBidiCharacters("")) // FSI
|
||||
assertEquals("$replacement", BidiUtil.replaceBidiCharacters("")) // PDI
|
||||
}
|
||||
|
||||
@Test
|
||||
fun replaceBidiCharacters_directionalIndicator_isReplaced() {
|
||||
assertEquals("$replacement", BidiUtil.replaceBidiCharacters("")) // RLM
|
||||
}
|
||||
|
||||
@Test
|
||||
fun replaceBidiCharacters_spoofedExtension_isNeutralized() {
|
||||
// "document<RLO>txt.exe<PDI>" would render as "documentexe.txt" without sanitization.
|
||||
val malicious = "documenttxt.exe"
|
||||
val cleaned = BidiUtil.replaceBidiCharacters(malicious)
|
||||
|
||||
assertEquals("document${replacement}txt.exe$replacement", cleaned)
|
||||
assertEquals(0, cleaned!!.count { it == '' || it == '' })
|
||||
}
|
||||
|
||||
@Test
|
||||
fun replaceBidiCharacters_multipleControls_allReplaced() {
|
||||
val input = "abcd"
|
||||
assertEquals("a${replacement}b${replacement}c${replacement}d", BidiUtil.replaceBidiCharacters(input))
|
||||
}
|
||||
}
|
||||
Loading…
Reference in New Issue
Block a user