Bug 568536 - asyncreq.c: Check return value from snprintf() for buffer overflow
Summary: asyncreq.c: Check return value from snprintf() for buffer overflow
Status: RESOLVED FIXED
Alias: None
Product: TCF
Classification: Tools
Component: Agent (show other bugs)
Version: unspecified   Edit
Hardware: Other other
: P3 normal (vote)
Target Milestone: 1.7   Edit
Assignee: Project Inbox CLA
QA Contact: Eugene Tarassov CLA
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-11-04 12:12 EST by Joel Sherrill CLA
Modified: 2021-06-25 16:23 EDT (History)
0 users

See Also:


Attachments
Check return value from snprintf() (808 bytes, patch)
2020-11-04 12:12 EST, Joel Sherrill CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joel Sherrill CLA 2020-11-04 12:12:45 EST
Created attachment 284671 [details]
Check return value from snprintf()

asyncreq.c has a potential buffer overflow where the return value from   snprintf() is not checked. Looking at the two values concatenated with a   / between them, it seemed safer to check the return value than attempt to make the destination buffer larger. This was especially true since one of the values is a simple char * with no obvious length limit.

   snprintf(path, sizeof(path), "%s/%s", req->u.dio.path, e->d_name);


d_name is part of struct dirent (dirent.h) and defined by POSIX as:

 	char	d_name[NAME_MAX + 1];	/* name must be no longer than this */

But the path field is defined as char * in asyncreq.h.

The length of the destination path variable would have to be (NAME_MAX+1) + 1 + unspecifed in length. It is clear the variable on the stack is not long enough but unclear how long it should be. It is also bad to have a huge buffer on the stack so checking for overflow was the safest option. It should be done even if the path variable's length is increased.
Comment 1 Eugene Tarassov CLA 2020-11-04 20:26:51 EST
It looks like wrong patch is attached.

Anyway, I completely agree the assessment. I have committed changes to check snprintf return value and use loc_printf when the buffer is not long enough.

Fixed.
Thanks!