Sonar has a Control tab. One of the things it does is switch the DNS resolver
for your Mac — tap Cloudflare, tap Quad9, or paste in the address of your own
Pi-hole. Changing a network service's DNS servers is a privileged operation, so
the app asks macOS for authorisation and runs networksetup on the other side
of that prompt.
That is a completely ordinary feature. It is also where I put a
command-injection that ran as root.
The shape of the bug
The custom-resolver field exists because people run Pi-hole and NextDNS on their
own networks, and hardcoding four public resolvers would have been useless to
them. So the field takes a string. The string goes into the arguments of a
privileged command. Between those two facts there was nothing at all.
The mental model that produced this is worth naming, because it is the one that
produces most injection bugs: it's my own app, and the field is for an IP
address, so it will contain an IP address. Both halves are wrong. The field is
for whatever the user types, and "my own app" is not a security boundary — the
whole point of asking for authorisation is that the code on the other side of
the prompt has more power than the code before it.
A privileged call is a trust boundary even when both sides are yours. Especially
when both sides are yours, because that is exactly when you stop looking.
Why "it's just a settings field" is not a defence
Two arguments came to mind while I was fixing it, and both are bad.
"The user would only be attacking themselves." Sometimes. But the value
that reaches that field does not have to be typed by the person sitting at the
machine. Anything that can put text into a field — a paste from a webpage, a
support article that says "paste this into the custom DNS box", an
accessibility-driven automation — becomes a way to run a command with elevated
privileges. Social engineering is a lot easier when the last step is paste this
string into the box and click Apply, and the app has already trained the user
to expect an admin prompt at that point.
"The authorisation prompt is the protection." The prompt authorises the
operation the app described, not whatever ends up in the argument list. The
user consenting to "change DNS servers" has not consented to anything else the
string can be made to mean. Consent obtained for one action does not cover a
different one that happens to travel through the same code path.
The fix, in the right order
The instinct is to reach for escaping — quote the string, strip the dangerous
characters, block a list of metacharacters. That is the wrong first move. Every
blocklist is a bet that you thought of everything, and you did not; that is what
a blocklist is.
Validate against what the value is allowed to be, then pass it in a way that
cannot be reinterpreted.
Allowlist the shape. A DNS server is an IPv4 or IPv6 address. That is a
tiny, completely specified grammar. Parse the string into an address type and
reject anything that does not parse — not "remove the bad characters", but
refuse the input. If it is not an address, there is no version of it that is
safe to pass along, so there is nothing to sanitise.
Never build a shell string. Pass arguments as an array to the process, so
there is no shell to interpret them and no quoting to get right. This is the
part people skip because a formatted string is quicker to write, and it is the
single highest-value habit in this whole area: if there is no shell, an
injection has nothing to inject into.
Keep the privileged surface small. The privileged side should accept the
narrowest possible instruction — "set the resolvers for this service to these
validated addresses" — not a command to run. The less expressive the interface
across the trust boundary, the less there is to abuse.
Those three in that order. Validation is the fix; the array-argument call is the
belt; the narrow interface is the braces.
The other twenty-five
I found this during a full pass over the app before shipping version 2, which
turned up twenty-six fixes. Three others are worth repeating because they are
the same category of mistake wearing different clothes.
Credentials in a plist. NextDNS and Pi-hole integrations need API tokens.
They were sitting in preferences, which is a plain file that any process running
as you can read, and which gets swept into backups and sync. They belong in the
Keychain, which is the one place on macOS designed to hold them. That is not a
hard fix; it is a fix nobody makes until they look.
CSV export as an attack. Sonar exports the network map. A device name is
attacker-controlled — anyone on your network can name their device. If a name
starts with =, a spreadsheet treats it as a formula when the file is opened.
Export is a place where your data becomes someone else's input, and that
direction gets almost no attention compared to import.
Corrupt-safe writes. The device history was written in place. A crash or a
full disk mid-write left a truncated file, and the next launch would read it,
fail, and start empty — silently destroying months of history. Writes are now
atomic, and a file that fails to parse is preserved rather than overwritten. The
distinction matters: losing data is bad, but losing it quietly means nobody
finds out until they need it.
What I would tell myself a version earlier
None of these were exotic. Every one is in the first chapter of anything written
about the subject, and I still shipped them, in a tool whose entire premise is
that it tells you when your network is exposed.
The reason is not ignorance, it is attention. Features get scrutiny while you
build them and then never again. The DNS field was written in an afternoon,
worked immediately, and was never reopened, because working code does not ask
for anything. A settings field that has never failed is the safest-feeling code
in the project and one of the most dangerous.
So the practice that actually helps is not a checklist. It is a question, asked
of the code you are least worried about: where does user input cross into
something with more power than the code that received it? Find those crossings
and look at every one, especially the boring ones you wrote in an afternoon and
never thought about again.
Sonar is free and open source under MIT, and all of this is in the history if
you want to read the diffs rather than take my word for it:
github.com/devShakib015/sonar.
Originally published at devshakib.jumyn.com. I write about Flutter, Dart and the parts of shipping that are genuinely awkward — and publish the packages that came out of them at pub.dev/publishers/jumyn.com.
Top comments (0)