Commit Graph

3 Commits (8975748527a82d2dcfd6168ada30359f39de91ba)

Author SHA1 Message Date
Tobias Nießen fcf27b12e3 test: use CHECK instead of EXPECT where necessary
GetPageSize() and OverrunGuardedBuffer currently use non-fatal EXPECT_*
macros because GoogleTest does not allow the fatal variants ASSERT_* in
non-void returning functions (i.e., in this file, nowhere outside of the
TEST itself).

The EXPECT_* macros continue execution upon failure, but we really don't
want that (and static analysis apparently does not like it either).
Since we cannot use GoogleTest's ASSERT_* here, use our own CHECK_*
instead of EXPECT_* outside of the TEST. Hopefully, this will finally
pacify static analysis.

Refs: https://github.com/nodejs/node/pull/44666

PR-URL: https://github.com/nodejs/node/pull/44795
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Erick Wendel <erick.workspace@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
2022-10-14 14:10:48 -04:00
Tobias Nießen 3492320fdd
test: check that sysconf returns a positive value
Static analysis insists that sysconf(_SC_PAGE_SIZE) might return a
negative integer (even though it never will). This was supposed to be
handled by the existing check EXPECT_GE(page, static_cast<int>(N)).
I assume that static analysis does not consider this sufficient because
static_cast<int>(N) could be negative or zero if N exceeds INT_MAX (even
though it never will).

To resolve this (theoretical) problem, explicitly check that the return
value is positive and then cast it to a size_t.

PR-URL: https://github.com/nodejs/node/pull/44666
Reviewed-By: Darshan Sen <raisinten@gmail.com>
2022-09-25 10:30:37 +00:00
Tobias Nießen a47c2c58ae
tls: fix out-of-bounds read in ClientHelloParser
ClientHelloParser::ParseHeader(data, avail) potentially accesses data
beyond avail bytes because it trusts the client to transmit a valid
frame length. Sending an impossibly small frame length causes the TLS
server to read beyond the buffer provided by the caller.

Guard against this by calling End() on the ClientHelloParser when the
client transmits an impossibly small frame length.

The test is designed to reliable cause a segmentation fault on Linux and
Windows when the buffer overrun occurs, and to trigger a spatial safety
violation when compiled with an address sanitizer enabled or when
running under valgrind.

PR-URL: https://github.com/nodejs/node/pull/44580
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
2022-09-15 17:27:04 +00:00