Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: Large amount of invisible unicode characters causes freeze and crash #3839

Closed
Kimapr opened this issue Feb 16, 2024 · 1 comment
Closed

Comments

@Kimapr
Copy link

Kimapr commented Feb 16, 2024

Problem description

Printing a specific file to terminal freezes and crashes Termux. The file is valid UTF-8 but contains large amounts of various invisible Unicode characters, including zero-width spaces, word joiners, variant selectors, etc.

Steps to reproduce the behavior.

  1. Download https://kimapr.net/lappy/matroska.js
  2. Print it to the terminal using cat or similar

What is the expected behavior?

Termux does not freeze or crash

System information

  • Termux application version: 0.118.0
  • Android OS version: 11
  • Device model: TECNO Spark 8C KG5j
tstein added a commit to tstein/termux-app that referenced this issue Feb 16, 2024
This prevents the crash in termux#3839, but not by enough to call it done.
Termux gets stuck on the very bad line and is responsive, but laggy
enough to trigger ANRs. It's possible to ^C in this state and get your
terminal back with no apparent damage.
@tstein
Copy link

tstein commented Feb 16, 2024

Sent #3840 with a fix for the crash, but there's still something asymptotically disastrous going on here. I gave it 15+ minutes trying to print the bad line before ^C.

agnostic-apollo added a commit to tstein/termux-app that referenced this issue Mar 21, 2024
… to prevent buffer overflows

The exception below causing app crash happens because of malicious input where combining characters keep getting added to same column of the row and this increases the size of `mSpaceUsed` and `mText`, eventually causing a buffer overflow of `mSpaceUsed`, which is limited to max `32767` value as per java `short` limit, but the limit itself isn't the issue, but an endless number of combining characters being added. Check `MAX_COMBINING_CHARACTERS_PER_COLUMN` field javadocs for why the limit `15` was chosen.

```
curl -o matroska.js https://kimapr.net/lappy/matroska.js
cat matroska.js
```

The `charCount` below refers to value of `Character.charCount(codePoint)`, like before `oldCharactersUsedForColumn` is appended to `newCharactersUsedForColumn`.

```
TerminalRow: codePoint=112, mColumns=98, mText=637, columnToSet=18, mSpaceUsed=590, javaCharDifference=0, oldStartOfColumnIndex=510, oldCharactersUsedForColumn=1, newCharactersUsedForColumn=1, oldNextColumnIndex=511, newNextColumnIndex=511, charCount=1, oldCodePointDisplayWidth=1, newCodePointDisplayWidth=1
TerminalRow: codePoint=40, mColumns=98, mText=637, columnToSet=19, mSpaceUsed=590, javaCharDifference=0, oldStartOfColumnIndex=511, oldCharactersUsedForColumn=1, newCharactersUsedForColumn=1, oldNextColumnIndex=512, newNextColumnIndex=512, charCount=1, oldCodePointDisplayWidth=1, newCodePointDisplayWidth=1
TerminalRow: codePoint=40, mColumns=98, mText=637, columnToSet=20, mSpaceUsed=590, javaCharDifference=0, oldStartOfColumnIndex=512, oldCharactersUsedForColumn=1, newCharactersUsedForColumn=1, oldNextColumnIndex=513, newNextColumnIndex=513, charCount=1, oldCodePointDisplayWidth=1, newCodePointDisplayWidth=1
TerminalRow: codePoint=101, mColumns=98, mText=637, columnToSet=21, mSpaceUsed=590, javaCharDifference=0, oldStartOfColumnIndex=513, oldCharactersUsedForColumn=1, newCharactersUsedForColumn=1, oldNextColumnIndex=514, newNextColumnIndex=514, charCount=1, oldCodePointDisplayWidth=1, newCodePointDisplayWidth=1
TerminalRow: codePoint=917772, mColumns=98, mText=147, columnToSet=18, mSpaceUsed=98, javaCharDifference=2, oldStartOfColumnIndex=18, oldCharactersUsedForColumn=1, newCharactersUsedForColumn=3, oldNextColumnIndex=19, newNextColumnIndex=21, charCount=2, oldCodePointDisplayWidth=1, newCodePointDisplayWidth=0
I TerminalRow: codePoint=65024, mColumns=98, mText=147, columnToSet=18, mSpaceUsed=100, javaCharDifference=1, oldStartOfColumnIndex=18, oldCharactersUsedForColumn=3, newCharactersUsedForColumn=4, oldNextColumnIndex=21, newNextColumnIndex=22, charCount=1, oldCodePointDisplayWidth=1, newCodePointDisplayWidth=0
TerminalRow: codePoint=917772, mColumns=98, mText=147, columnToSet=18, mSpaceUsed=101, javaCharDifference=2, oldStartOfColumnIndex=18, oldCharactersUsedForColumn=4, newCharactersUsedForColumn=6, oldNextColumnIndex=22, newNextColumnIndex=24, charCount=2, oldCodePointDisplayWidth=1, newCodePointDisplayWidth=0
...
TerminalRow: codePoint=917959, mColumns=98, mText=32781, columnToSet=18, mSpaceUsed=32763, javaCharDifference=2, oldStartOfColumnIndex=18, oldCharactersUsedForColumn=32666, newCharactersUsedForColumn=32668, oldNextColumnIndex=32684, newNextColumnIndex=32686, charCount=2, oldCodePointDisplayWidth=1, newCodePointDisplayWidth=0
TerminalRow: codePoint=917939, mColumns=98, mText=32781, columnToSet=18, mSpaceUsed=32765, javaCharDifference=2, oldStartOfColumnIndex=18, oldCharactersUsedForColumn=32668, newCharactersUsedForColumn=32670, oldNextColumnIndex=32686, newNextColumnIndex=32688, charCount=2, oldCodePointDisplayWidth=1, newCodePointDisplayWidth=0
TerminalRow: codePoint=917961, mColumns=98, mText=32781, columnToSet=18, mSpaceUsed=32767, javaCharDifference=2, oldStartOfColumnIndex=18, oldCharactersUsedForColumn=32670, newCharactersUsedForColumn=32672, oldNextColumnIndex=32688, newNextColumnIndex=32690, charCount=2, oldCodePointDisplayWidth=1, newCodePointDisplayWidth=0
TerminalRow: codePoint=917804, mColumns=98, mText=32781, columnToSet=18, mSpaceUsed=-32767, javaCharDifference=2, oldStartOfColumnIndex=18, oldCharactersUsedForColumn=1, newCharactersUsedForColumn=3, oldNextColumnIndex=19, newNextColumnIndex=21, charCount=2, oldCodePointDisplayWidth=1, newCodePointDisplayWidth=0
```

```
java.lang.ArrayIndexOutOfBoundsException: src.length=32781 srcPos=19 dst.length=32781 dstPos=21 length=-32786
	at java.lang.System.arraycopy(System.java:469)
	at com.termux.terminal.TerminalRow.setChar(TerminalRow.java:196)
	at com.termux.terminal.TerminalBuffer.setChar(TerminalBuffer.java:455)
	at com.termux.terminal.TerminalEmulator.emitCodePoint(TerminalEmulator.java:2380)
	at com.termux.terminal.TerminalEmulator.processCodePoint(TerminalEmulator.java:624)
	at com.termux.terminal.TerminalEmulator.processByte(TerminalEmulator.java:520)
	at com.termux.terminal.TerminalEmulator.append(TerminalEmulator.java:487)
	at com.termux.terminal.TerminalSession$MainThreadHandler.handleMessage(TerminalSession.java:358)
	at android.os.Handler.dispatchMessage(Handler.java:106)
	at android.os.Looper.loop(Looper.java:223)
	at android.app.ActivityThread.main(ActivityThread.java:7664)
	at java.lang.reflect.Method.invoke(Native Method)
	at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:592)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:947)
```

See also following links for history of related changes to `TerminalRow` for combining characters. Note that jackpal terminal does not crash for above, which termux-app is based on, but changes were done by fornwall in initial commit of termux-app to change the behaviour, hence the crash, but he added the `FIXME: Put a limit of combining characters` comment as a note to solve the current issue in future, which is now.

- jackpal/Android-Terminal-Emulator@9a47042
- jackpal/Android-Terminal-Emulator#338
- termux@a18ee58#diff-f84d215b18106c037e01986a3968fa54b74691174a78fcc99493f745d3805be5

Closes termux#3839
agnostic-apollo added a commit to tstein/termux-app that referenced this issue Apr 7, 2024
… to prevent buffer overflows

The exception below causing app crash happens because of malicious input where combining characters keep getting added to same column of the row and this increases the size of `mSpaceUsed` and `mText`, eventually causing a buffer overflow of `mSpaceUsed`, which is limited to max `32767` value as per java `short` limit, but the limit itself isn't the issue, but an endless number of combining characters being added. Check `MAX_COMBINING_CHARACTERS_PER_COLUMN` field javadocs for why the limit `15` was chosen.

```
curl -o matroska.js https://kimapr.net/lappy/matroska.js
cat matroska.js
```

The `charCount` below refers to value of `Character.charCount(codePoint)`, like before `oldCharactersUsedForColumn` is appended to `newCharactersUsedForColumn`.

```
TerminalRow: codePoint=112, mColumns=98, mText=637, columnToSet=18, mSpaceUsed=590, javaCharDifference=0, oldStartOfColumnIndex=510, oldCharactersUsedForColumn=1, newCharactersUsedForColumn=1, oldNextColumnIndex=511, newNextColumnIndex=511, charCount=1, oldCodePointDisplayWidth=1, newCodePointDisplayWidth=1
TerminalRow: codePoint=40, mColumns=98, mText=637, columnToSet=19, mSpaceUsed=590, javaCharDifference=0, oldStartOfColumnIndex=511, oldCharactersUsedForColumn=1, newCharactersUsedForColumn=1, oldNextColumnIndex=512, newNextColumnIndex=512, charCount=1, oldCodePointDisplayWidth=1, newCodePointDisplayWidth=1
TerminalRow: codePoint=40, mColumns=98, mText=637, columnToSet=20, mSpaceUsed=590, javaCharDifference=0, oldStartOfColumnIndex=512, oldCharactersUsedForColumn=1, newCharactersUsedForColumn=1, oldNextColumnIndex=513, newNextColumnIndex=513, charCount=1, oldCodePointDisplayWidth=1, newCodePointDisplayWidth=1
TerminalRow: codePoint=101, mColumns=98, mText=637, columnToSet=21, mSpaceUsed=590, javaCharDifference=0, oldStartOfColumnIndex=513, oldCharactersUsedForColumn=1, newCharactersUsedForColumn=1, oldNextColumnIndex=514, newNextColumnIndex=514, charCount=1, oldCodePointDisplayWidth=1, newCodePointDisplayWidth=1
TerminalRow: codePoint=917772, mColumns=98, mText=147, columnToSet=18, mSpaceUsed=98, javaCharDifference=2, oldStartOfColumnIndex=18, oldCharactersUsedForColumn=1, newCharactersUsedForColumn=3, oldNextColumnIndex=19, newNextColumnIndex=21, charCount=2, oldCodePointDisplayWidth=1, newCodePointDisplayWidth=0
I TerminalRow: codePoint=65024, mColumns=98, mText=147, columnToSet=18, mSpaceUsed=100, javaCharDifference=1, oldStartOfColumnIndex=18, oldCharactersUsedForColumn=3, newCharactersUsedForColumn=4, oldNextColumnIndex=21, newNextColumnIndex=22, charCount=1, oldCodePointDisplayWidth=1, newCodePointDisplayWidth=0
TerminalRow: codePoint=917772, mColumns=98, mText=147, columnToSet=18, mSpaceUsed=101, javaCharDifference=2, oldStartOfColumnIndex=18, oldCharactersUsedForColumn=4, newCharactersUsedForColumn=6, oldNextColumnIndex=22, newNextColumnIndex=24, charCount=2, oldCodePointDisplayWidth=1, newCodePointDisplayWidth=0
...
TerminalRow: codePoint=917959, mColumns=98, mText=32781, columnToSet=18, mSpaceUsed=32763, javaCharDifference=2, oldStartOfColumnIndex=18, oldCharactersUsedForColumn=32666, newCharactersUsedForColumn=32668, oldNextColumnIndex=32684, newNextColumnIndex=32686, charCount=2, oldCodePointDisplayWidth=1, newCodePointDisplayWidth=0
TerminalRow: codePoint=917939, mColumns=98, mText=32781, columnToSet=18, mSpaceUsed=32765, javaCharDifference=2, oldStartOfColumnIndex=18, oldCharactersUsedForColumn=32668, newCharactersUsedForColumn=32670, oldNextColumnIndex=32686, newNextColumnIndex=32688, charCount=2, oldCodePointDisplayWidth=1, newCodePointDisplayWidth=0
TerminalRow: codePoint=917961, mColumns=98, mText=32781, columnToSet=18, mSpaceUsed=32767, javaCharDifference=2, oldStartOfColumnIndex=18, oldCharactersUsedForColumn=32670, newCharactersUsedForColumn=32672, oldNextColumnIndex=32688, newNextColumnIndex=32690, charCount=2, oldCodePointDisplayWidth=1, newCodePointDisplayWidth=0
TerminalRow: codePoint=917804, mColumns=98, mText=32781, columnToSet=18, mSpaceUsed=-32767, javaCharDifference=2, oldStartOfColumnIndex=18, oldCharactersUsedForColumn=1, newCharactersUsedForColumn=3, oldNextColumnIndex=19, newNextColumnIndex=21, charCount=2, oldCodePointDisplayWidth=1, newCodePointDisplayWidth=0
```

```
java.lang.ArrayIndexOutOfBoundsException: src.length=32781 srcPos=19 dst.length=32781 dstPos=21 length=-32786
	at java.lang.System.arraycopy(System.java:469)
	at com.termux.terminal.TerminalRow.setChar(TerminalRow.java:196)
	at com.termux.terminal.TerminalBuffer.setChar(TerminalBuffer.java:455)
	at com.termux.terminal.TerminalEmulator.emitCodePoint(TerminalEmulator.java:2380)
	at com.termux.terminal.TerminalEmulator.processCodePoint(TerminalEmulator.java:624)
	at com.termux.terminal.TerminalEmulator.processByte(TerminalEmulator.java:520)
	at com.termux.terminal.TerminalEmulator.append(TerminalEmulator.java:487)
	at com.termux.terminal.TerminalSession$MainThreadHandler.handleMessage(TerminalSession.java:358)
	at android.os.Handler.dispatchMessage(Handler.java:106)
	at android.os.Looper.loop(Looper.java:223)
	at android.app.ActivityThread.main(ActivityThread.java:7664)
	at java.lang.reflect.Method.invoke(Native Method)
	at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:592)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:947)
```

See also following links for history of related changes to `TerminalRow` for combining characters. Note that jackpal terminal does not crash for above, which termux-app is based on, but changes were done by fornwall in initial commit of termux-app to change the behaviour, hence the crash, but he added the `FIXME: Put a limit of combining characters` comment as a note to solve the current issue in future, which is now.

- jackpal/Android-Terminal-Emulator@9a47042
- jackpal/Android-Terminal-Emulator#338
- termux@a18ee58#diff-f84d215b18106c037e01986a3968fa54b74691174a78fcc99493f745d3805be5

Closes termux#3839
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants