Skip to content

Commit

Permalink
libnpfs: fix off by one buffer check
Browse files Browse the repository at this point in the history
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
  • Loading branch information
garlick committed Dec 29, 2024
1 parent 756fff7 commit 7161d1d
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 1 deletion.
2 changes: 1 addition & 1 deletion libnpfs/np.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions tests/misc/t07.exp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
26 changes: 26 additions & 0 deletions tests/misc/tserialize.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -112,6 +114,8 @@ main (int argc, char *argv[])
test_tclunk (); test_rclunk ();
test_tremove (); test_rremove ();

test_truncated_tlink ();

exit (0);
}

Expand Down Expand Up @@ -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
*/

0 comments on commit 7161d1d

Please sign in to comment.