the parsers for all modes except the command line (which has
always been protected inside an iframe).
If the native messenger was not installed, the bug could not be
exploited for any more than nuisance attacks (closing tabs,
quitting Firefox, etc.). If the native messenger was installed,
an attack using the mpv hint mode (bound to `;v` by default) and
a specially crafted link would allow an attacker to execute some
commands in the user's shell. Due to the way hyperlinks are
encoded, it would require more cunning than the Tridactyl
developers possess to usefully exploit as it is difficult to pass
arguments to commands.
This did mean that the standard output of mpv (including the
attacker's URL) was also available to an attacker via pipes. We
are not aware of any way to abuse that with commonly installed
utilities.
We are unaware of any pages exploiting this in the wild.
Nevertheless, this security regression should not have happened.
A short incident report follows:
These checks were accidentally removed when key handling was
rewritten in September 2018. The PR was reviewed, but it was a
large PR and the regression was missed by the reviewers.
We became aware of the regression after a question in our support
chat prompted @glacambre to check on exactly how we were using
`isTrusted` and they realised that we weren't using it any more.
We will shortly introduce automated testing to check these
security properties that we rely on.
We will consider adding a check to continuous integration that
flags any change to files containing security relevant code for
more detailed review.
Affected versions: - Tridactyl 1.14.0 - 1.14.10, 1.15.0.
Mitigation:
- Update to Tridactyl 1.16.0+ or 1.14.13+
- If updating is unfeasible, we recommend removing the native
messenger by running `:! pwd` in Tridactyl and then deleting that
directory from your filesystem.
- If you've thought of a clever exploit, please contact
bovine3dom or cmcaine privately on Matrix or by email.
Previously they went on a magical mystery tour with up to
four stops on the way.
Now they take one synchronous step and are done, hopefully
avoiding race conditions.
BGSELF was a hack that I used when implementing ex commands for the
command line. It consisted of having .excmds_background.generated.ts
import itself as BGSELF in order to dynamically add commands to it. This
let us define excmds in other files while not changing anything in
parsers/exmode.ts.
This was awful so I decided to remove it. This required performing the
following changes:
- Moving text.* and ex.* command definitions to their own files where
they have zero side effects. While this was easy for text commands, ex
commands rely a lot on side effects. In order to work around this,
lib/commandline_cmds exports a single function, getCommandlineFns(),
which expects an object representing the commandline's state as
parameter.
- In the background script, import our side effect free files and wrap
them in proxys that will send "commandline_cmd" and "editorfn_content"
messages to tabs when needed.
- In the content script, add a listener that will either execute an
editor function or forward it to the command line when it receives an
"editorfn_content" message.
- In the commandline script, add a listener that will execute exmode
commands.
This rule requires adding a new set of rules, tslint-etc.
no-unused-declaration used to be available in tslint:recommended but was
deprecated when --noUnusedVariables was added to typescript. The problem
with using TypeScript's --noUnusedVariables is that it turns unused
declarations into an error and prevents compilation, which isn't fun
when you're just prototyping things.
Before this commit, Tridactyl had a bug where resizeArea could be called
before completion computation had ended, which resulted in completions
pushing the input field out of the viewport (easy way to reproduce this:
open a lot of tabs and press `b` to open buffer completions).
This happened because for some of the completion sources, `filter`
returned before completion computation had actually ended. This is fixed
by making sure that filter() (and all underlying calls to updateOptions,
onInput, updateChain...) return a promise that will only be resolved
once completion computation has actually ended.
One of the problems of the command line was that it made a resizeArea()
call for each enabled completion, no matter whether its status was
"hidden" or "normal". This was a problem because a resizeArea call
results in 2 cross-script messages: a "show" and a "focus" message. This
means that for each keystroke, we sent 28 messages. This commit fixes
that thanks to modifications in multiple files:
- commandline_frame.ts: Stop accumulating event listeners on resizeArea
calls. Make sure completion sources actually need a refresh before
calling resizeArea().
- completions.ts: Add logic to know whether a completion source needs a
refresh or not.
- {Rss,Sessions,Tab,TabAll,Window}.ts: Make sure that completions are
actually needed before computing them.
This seems to make opening the command line slightly faster for me,
although I can't tell if this is placebo or not.
This commit removes commandline_background.ts. I believe this is useful
because the only thing it did was provide recvExStr, which just
triggered a synthetic "onLine" event the only consumer of which was the
parser. Since we already used controller_background + acceptExCmd in
some places, it made sense to me to directly use controller_background +
acceptExCmd everywhere.
10cb692d72 broke the build bot by
introducing type incompatibilites. This is fixed by making sure .catch()
returns an array.
This enables removing the try/catch in the input event handler since
the await won't throw anymore.
https://github.com/tridactyl/tridactyl/issues/1345 describes a problem
where completion computation fails and fills the command line with an
error message.
This commit doesn't make the underlying problem disappear but prevents
Tridactyl from spamming the command line.
https://github.com/tridactyl/tridactyl/issues/1329 describes a bug that
can be triggered with the following key presses:
:t<ArrowUp><ArrowDown><Backspace>w<ArrowUp>
This results in the commandline being filled with a command starting
with `t` rather than `w`. This is caused by not updating the
HISTORY_SEARCH_STRING variable on changes and is fixed by always
resetting it if the previous command did not call history().
Closes#1329.
https://github.com/tridactyl/tridactyl/issues/1295 reports that
sometimes, completions won't be offered for excmds even though they
should. This happened because of the following steps:
- A letter is pressed, triggering an "input" event which schedules
completion computation
- <Space> is pressed, which doesn't trigger an "input" event since it's
bound to an excmd. However, the excmd itself refreshes completions.
- The computation scheduled by the "input" event is run with the
previous exstr, even though it isn't up to date anymore.
This is fixed by replacing all the complicated timeoutId checking with
exstr checking, which makes a lot more sense, is simpler and all around
better.
Closes#1295.
Winclose was already there but was much less useful (could only close
the current window). This commit enables closing other windows and
provides completions for it.
Related issue: https://github.com/tridactyl/tridactyl/issues/794.
The previous code simulated an input event in order to trigger the input
event handler which recomputed completions. This was ok until delays
were added to the input event handlers in order to reduce the lag that
could happen when typing fast/keeping a key pressed. This delay also
affects completion computation on other actions, such as fillcmdline.
In order to remove this delay, we move completion computation out of the
event handler and directly call this functions everywhere we previously
triggered an input event.
This should help with https://github.com/tridactyl/tridactyl/issues/1242
PR#1183 broke completions for users that had a tab -> buffer alias. This
is because having such an alias created a loop, which made
BufferCompletionSource throw errors when being instanciated, which
resulted in the activeCompletions array not being created.
This is fixed in two ways: first, a config updater is created in order
to remove the alias. Second, completions are instantiated in
try/catchs, which should hopefully prevent a faulty completion source
constructor from breaking every completion source.
This is a temporary fix for
https://github.com/tridactyl/tridactyl/issues/1167 . I couldn't find
where this issue might come from and since there are no reproduction
steps this will have to do.
I plan on modeling Tridactyl in TLA+ when I have time in order to find
out where our hard-to-reproduce bugs come from. Hopefully this bug will
be among them.
Closes#1167.
As cmcaine said in #1149, the help page links are currently broken
because typedoc tries to generate documentation for source files in the
`compiler/` directory. I just realized that before #1026, these files
were not referenced in any of the files in the `src` directory and this
is why typedoc ignored them. This change happened because I wanted to
type the metadata.
There are three possible solution to #1149.
- Go back to untyped metadata.
- Move the metadata types to Tridactyl's src directory on build.
- Update all links to the doc.
I believe having typed metadata is useful and I'd like to keep it that
way. Moving the metadata types to Tridactyl's src directory is certainly
doable but doesn't sound like the best idea to me, we're unnecessarily
copying files. Updating the links to the doc sounds reasonable as it's
only a one-time thing and so this is what this commit does in order to
close#1149.
This commit makes error messages when the native messenger is
unavailable easier to read. Since they're easier to read, there's no
need for custom errors in setclip/getclip anymore, provided that the
errors they throw are correctly logged. In order to make sure of that,
we remove the try/catch in excmds.ts:clipboard(), which should let
errors bubble up as needed.
I also noticed that while {set,get}Clipboard relied on the command line
being focused in order to work, they didn't do that themselves and
instead expected their callers to have set things up. This didn't make
sense to me so I moved the focusing code inside of {set,get}Clipboard.
This was all done while chasing the elusive #1135 but probably doesn't
change anything about it.
Before this commit, keyEvents was never reset. This caused weird issues
like control closing the commandline after a first ex command had been
accepted. We now reset keyEvents in two places ; when an ex command is
accepted (which could happen when inserting a completion, for example)
and when the command line is closed (which could be triggered by an
outside event).
We re-use the ex-command parser in order to enable rebinding keys in
command-line mode. Command-line manipulation is re-implemented using
previously-abstracter editor commands.
