From 7161d1dc04390119e4512291051d333c08f77d8e Mon Sep 17 00:00:00 2001 From: Jim Garlick Date: Sat, 28 Dec 2024 11:42:11 -0800 Subject: [PATCH] libnpfs: fix off by one buffer check Problem: buf_check_size() doesn't flag a buffer overflow if the buffer is exactly full when the check is performed. For example, a malformed Tlink with name size > 0 but no name causes a segfault. Fix off by one check so this and similar malformed requests generate an error response. Add unit test. Fixes #109 --- libnpfs/np.c | 2 +- tests/misc/t07.exp | 1 + tests/misc/tserialize.c | 26 ++++++++++++++++++++++++++ 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/libnpfs/np.c b/libnpfs/np.c index 7485f9df..5ccd0af2 100644 --- a/libnpfs/np.c +++ b/libnpfs/np.c @@ -47,7 +47,7 @@ static inline int buf_check_size(struct cbuf *buf, int len) { if (buf->p+len > buf->ep) { - if (buf->p < buf->ep) + if (buf->p <= buf->ep) buf->p = buf->ep + 1; return 0; diff --git a/tests/misc/t07.exp b/tests/misc/t07.exp index cc289df5..cd87af7a 100644 --- a/tests/misc/t07.exp +++ b/tests/misc/t07.exp @@ -122,3 +122,4 @@ test_tremove(122): 11 P9_TREMOVE tag 42 fid 1 test_rremove(123): 7 P9_RREMOVE tag 42 +OK np_deserialize failed on truncated Tlink (issue#109) diff --git a/tests/misc/tserialize.c b/tests/misc/tserialize.c index 95a6e449..97662a8f 100644 --- a/tests/misc/tserialize.c +++ b/tests/misc/tserialize.c @@ -65,6 +65,8 @@ static void test_twrite (void); static void test_rwrite (void); static void test_tclunk (void); static void test_rclunk (void); static void test_tremove (void); static void test_rremove (void); +static void test_truncated_tlink (void); + static void usage (void) { @@ -112,6 +114,8 @@ main (int argc, char *argv[]) test_tclunk (); test_rclunk (); test_tremove (); test_rremove (); + test_truncated_tlink (); + exit (0); } @@ -1194,6 +1198,28 @@ test_rremove (void) free (fc2); } +static void test_truncated_tlink (void) +{ + Npfcall *fc; + + if (!(fc = np_create_tlink (1, 2, "xyz"))) + msg_exit ("out of memory"); + + // truncate fc->pkt so entire name is missing + fc->size -= 3; + fc->pkt[0] = fc->size; + fc->pkt[1] = fc->size >> 8; + fc->pkt[2] = fc->size >> 16; + fc->pkt[3] = fc->size >> 24; + + if (np_deserialize (fc) == 0) + printf ("OK np_deserialize failed on truncated Tlink (issue#109)\n"); + else + printf ("FAIL np_deserialize worked on truncated Tlink (issue#109)\n"); + + free (fc); +} + /* * vi:tabstop=4 shiftwidth=4 expandtab */