[asterisk-bugs] [Zaptel 0011471]: [patch] unsafe use of strncpy

[noreply at bugs.digium.com]

A NOTE has been added to this issue. 
====================================================================== 
http://bugs.digium.com/view.php?id=11471 
====================================================================== 
Reported By:                flefoll
Assigned To:                tzafrir
====================================================================== 
Project:                    Zaptel
Issue ID:                   11471
Category:                   General
Reproducibility:            always
Severity:                   minor
Priority:                   normal
Status:                     assigned
Zaptel Version:              SVN 
SVN Branch (only for SVN checkouts, not tarball releases):  1.4  
SVN Revision (number only!): 3317 
Disclaimer on File?:        N/A 
Request Review:              
====================================================================== 
Date Submitted:             12-05-2007 03:15 CST
Last Modified:              12-14-2007 05:47 CST
====================================================================== 
Summary:                    [patch] unsafe use of strncpy
Description: 
strncpy(dst, src, len) makes a copy of src towards dst, limited to len
characters, but does NOT guarantee a final null character. So if you want
to obtain a "traditional" null-terminated string, extra work is required.

Zaptel uses strncpy in many places, but most often does not do this extra
work, while destination initial contents is undetermined (stack, ...).

I suggest to use snprintf() instead of strncpy(), since snprintf does
guarantee a final null character. Constraint : snprintf requires including
stdio.h, while strncpy requires including strings.h.

See additional information for the list of files that use strncpy.

I will upload patches for a number of them (not all at the moment).

The same work should be done on SVN Trunk, but it did not compile
yesterday for me (Fedora 7)
====================================================================== 

---------------------------------------------------------------------- 
 flefoll - 12-14-07 05:47  
---------------------------------------------------------------------- 
I uploaded a new version of the patches for zaptel 1.4.7.1, with a
zap_copy_string() static inline function defined in order to avoid using
snprintf().

zap_copy_string() is a copy/paste of Asterisk ast_copy_string(), except
'size' type that is 'unsigned int' instead of 'size_t'.

However, zap_copy_string() is not used everywhere :
- The patch still uses a snprintf in order to replace a
sprintf+strncpy+strncat in zaptel-base.c
- The patch just adds a forced buf[lastbyte]=0 in a few places where
zaptel.h is not known.

What the patch does NOT do :
- I guess that returning a constant string in all
xxx.h:echo_can_identify(), instead of doing the copy operation there, would
be cleaner. But I did not change this.
- There remains plenty of sprintf() or strcpy() that are possibly not
safer than a misused strncpy() :-) ! I guess that these should be replaced
as well. Not done here. 

Issue History 
Date Modified   Username       Field                    Change               
====================================================================== 
12-14-07 05:47  flefoll        Note Added: 0075381                          
======================================================================