Comment 9 for bug 1997417

Revision history for this message
Seth Arnold (seth-arnold) wrote :

I reviewed duktape 2.7.0-1 as checked into kinetic. This shouldn't be
considered a full audit but rather a quick gauge of maintainability.
This is an exceedingly complex package with very intricate inner workings,
we will be highly reliant upon upstream to prepare fixes.

duktape is an embeddable javascript interpreter; it's intended for uses
more like Lua than like v8.

- CVE History:
  - CVE-2021-46322 -- the upstream author communicated results along the
    way, came up with a small fix, and included a test case with the pull
    request. Nice.
- Build-Depends?
  - debhelper-compat, dh-exec
- pre/post inst/rm scripts?
  - None
- init scripts?
  - None
- systemd units?
  - None
- dbus services?
  - None
- setuid binaries?
  - None
- binaries in PATH?
  - /usr/bin/duk
- sudo fragments?
  - None
- polkit files?
  - none
- udev rules?
  - None
- unit tests / autopkgtests?
  - The upstream tests have been removed
  - No autopkgtests
  - This is very unfortunate
- cron jobs?
  - None
- Build logs:
  - clean

- Processes spawned?
  - None
- Memory management?
  - Extremely complicated. I've got concerns.
- File IO?
  - None
- Logging?
  - Spot checks looked fine
- Environment variable usage?
  - None
- Use of privileged functions?
  - None
- Use of cryptography / random number sources etc?
  - None
- Use of temp files?
  - None
- Use of networking?
  - None
- Use of WebKit?
  - None
- Use of PolicyKit?
  - Funny enough... this *is* polkit, now :)

- Any significant cppcheck results?
  - pointless memory leaks in example
- Any significant Coverity results?
  - findings in debugger/ examples/ and polyfills/ -- I didn't inspect
- Any significant shellcheck results?
  - None
- Any significant bandit results?
  - Unsafe yaml load() function in tools/

This is very complex software. The memory management is very intricate,
and almost certainly has flaws. One defining feature of the program is
that it 100% trusts the input code and any stored bytecode. This is a fine
policy decision to make, and the upstream author has articulated it very
well, but it does drastically limit where duktape makes sense to be used.

As a general sketch, polkit files are fine. They are supplied by package
maintainers (who have root) or system administrators (who have root). They
MUST be perfect anyway.

Gnome extensions are iffy. The code quality of these are all over the
place, they're often run by users without review, but even if duktape were
perfect software, that'd still be a very dangerous thing to do. So it's
probably fine but not perfect.

JavaScript content loaded over the web is not appropriate. v8 has no end
of flaws in this threat model and I suspect duktape would be similar.

When I raised my concerns about memory allocations and integer overflows
very early in my review, the upstream author wrote back a very excellent
reply that addressed my concerns completely. We'd be happy to work with
the upstream author again but I suspect the threat model means that very
few bugs will actually be security bugs.

Security team ACK for promoting duktape to main.