[Bug nscd/30795] New: avoid snprintf using %n to generate coredump when F_S=2 is enabled

["zhanghao383 at huawei dot com" <sourceware-bugzilla@sourceware.org>]

https://sourceware.org/bugzilla/show_bug.cgi?id=30795

            Bug ID: 30795
           Summary: avoid snprintf using %n to generate coredump when
                    F_S=2 is enabled
           Product: glibc
           Version: 2.34
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: nscd
          Assignee: unassigned at sourceware dot org
          Reporter: zhanghao383 at huawei dot com
                CC: drepper.fsp at gmail dot com
  Target Milestone: ---

Created attachment 15084
  --> https://sourceware.org/bugzilla/attachment.cgi?id=15084&action=edit
coredump details

Recently, we found that two coredump occurred when nscd involved calling the
snprintf function and using %n and F_S=2 is set, the following two call stacks: 
and give the following prompt:
*** %n in writable segment detected ***
And the input parameters of the two call stacks look normal.
Involved version: glibc 2.34

We use a simple test case to verify it:
#include <stdio.h>
#include <string.h>
int main ()
{
  char fmtstring[10];
  char buf[100];
  int count = -1;
  strcpy (fmtstring, "%d%n");
  snprintf (buf, 100, fmtstring, 123, &count);
  return 0;
}
when compiling with
gcc snprintf_test.c -fstack-protector -Wall -Wformat -Wformat-security
-D_FORTIFY_SOURCE=2 -O2 -o snprintf_test -g
./ snprintf_test
*** %n in writable segment detected ***
Aborted (core dumped)

when compiling with
gcc snprintf_test.c -fstack-protector -Wall -Wformat -Wformat-security -O2 -o
snprintf_test -g
./ snprintf_test
no core dumped

We strip the calculation logic outside the snprintf function for replacement:
From 4816192ca348e55b7b1d33feac9298d5b0ffb04c Mon Sep 17 00:00:00 2001
From: zhanghao<zhanghao383@huawei.com>
Date: Mon, 21 Aug 2023 15:39:56 +0800
Subject: [PATCH] Avoid snprintf using %n to generate coredump when F_S=2 is
enabled

In nscd, F_S=2 added in 233399bce2e79e5af3b344782e9943d5f1a9cdcb just for
warn_if_unused
warnings rather than anything substantial.

When F_S=2 is set, and snprintf() using %n will generate coredump and give the
following prompt:

*** %n in writable segment detected ***

It is not recommended to use %n to calculate the length of the string in the
snprintf function. We strip the calculation logic outside the snprintf function
for replacement.

---
nscd/grpcache.c | 5 +++--
nscd/pwdcache.c | 5 +++--
2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/nscd/grpcache.c b/nscd/grpcache.c
index 457ca4d8..d7200f4e 100644
--- a/nscd/grpcache.c
+++ b/nscd/grpcache.c
@@ -176,8 +176,9 @@ cache_addgr (struct database_dyn *db, int fd,
request_header *req,

       /* We need this to insert the `bygid' entry.  */
       int key_offset;
-      n = snprintf (buf, buf_len, "%d%c%n%s", grp->gr_gid, '\0',
-                   &key_offset, (char *) key) + 1;
+      n = snprintf (buf, buf_len, "%d%c%s", grp->gr_gid, '\0',
+                   (char *) key) + 1;
+      key_offset = n - strlen((char *) key)- 1;

       /* Determine the length of all members.  */
       while (grp->gr_mem[gr_mem_cnt])
diff --git a/nscd/pwdcache.c b/nscd/pwdcache.c
index dfafb526..37dd402f 100644
--- a/nscd/pwdcache.c
+++ b/nscd/pwdcache.c
@@ -180,8 +180,9 @@ cache_addpw (struct database_dyn *db, int fd,
request_header *req,

       /* We need this to insert the `byuid' entry.  */
       int key_offset;
-      n = snprintf (buf, buf_len, "%d%c%n%s", pwd->pw_uid, '\0',
-                   &key_offset, (char *) key) + 1;
+      n = snprintf (buf, buf_len, "%d%c%s", pwd->pw_uid, '\0',
+                   (char *) key) + 1;
+      key_offset = n - strlen((char *) key) - 1;

       total = (offsetof (struct dataset, strdata)
               + pw_name_len + pw_passwd_len
--
2.33.0

-- 
You are receiving this mail because:
You are on the CC list for the bug.