Comments (12)
Imported from trac issue 35. Created by glimes on 2015-04-10T15:30:24, last modified: 2019-08-14T14:11:46
from osal.
Trac comment by glimes on 2015-06-10 14:45:17:
This could very well go away with #40 (or similar) changes.
Documenting the bad strncpy
usage, and placing this ticket
on ''hold'' until the #40 vs UT-Assert issues are resolved.
Confirming that as of [changeset:2c9c3451] ic-2015-0602-merge
the following incorrect usages of strncpy()
still exist in
the OSAL Unit Test code:
ut_os_stubs.c
-- UT_os_log_api()
various lines:
{{{
strncpy(testInfo->name, testPtr->name, strlen(testPtr->name));
strncpy(testInfo->name, testPtr->name, strlen(testPtr->name));
strncpy(testInfo->result, testPtr->result, strlen(testPtr->result));
}}}
The 3rd argument is the length of the destination buffer,
so that strncpy will stop before it overflows. It already
knows to stop at the end of the source data (after moving
the zero termination), so the net result of the above is to
always copy all of the name to the buffer, even if it overflows,
and to never copy the terminating NUL
.
Refraining from copying the NUL
is an issue here, as the
structure contining the field which is being filled has been
pre-cleared to the NUL
value, however, this usage actively
obscures the fact that the buffer overflow protection in strncpy
is actively being broken.
Here, the 3rd parameter should be sizeof (testInfo->name)
or sizeof (testInfo->result)
as apporpriate. We know these
fields are arrays, not pointers, because the memset
would have
nulled any pointers present.
ut_os_stubs.h
-- various macros:
{{{
strncpy(VAR1.name, APINAME, strlen(APINAME));
strncpy(VAR1.tests[TSTIDX].name, TSTNAME, strlen(TSTNAME));
strncpy(VAR1.tests[TSTIDX].result, TSTRESULT, strlen(TSTRESULT));
}}}
Comments as above. 3rd arg needs to be ''SIZE OF DESTINATION BUFFER'',
not ''LENGTH OF THE SOURCE STRING''.
from osal.
Trac comment by glimes on 2015-07-13 18:16:59:
When updating code using strncpy
bear in mind the exact wording of the language specification:
{{{
#include <string.h>
char *strncpy(char * restrict s1,
const char * restrict s2,
size_t n);
}}}
The strncpy
function copies not more than ''n'' characters (characters that follow a \0
character are not copied) from the array pointed to by ''s2'' to the array pointed to by ''s1''. If copying takes place between objects that overlap, the behavior is undefined.
If the array pointed to by ''s2'' is a string that is shorter than ''n'' characters, null characters are appended to the copy in the array pointed to by ''s1'', until ''n'' characters in all have been written.
That last paragraph was something I'd forgotten. It might be useful, and most assuredly obsoletes code that explicitly clears buffers before using strncpy
to fill them. Just thought I would throw this into the ticket. I doubt that I'm the only person who forgets the fact that strncpy
zero-fills the rest of the destination after short strings.
from osal.
Trac comment by glimes on 2015-07-13 18:43:03:
Remaining misuses of strncpy
in the most bleeding edge version of OSAL in Babelfish:
-
a bunch of places in os/vxworks6 the size is 32
when an appropriate OS_MAX_something macro should be used. -
rtems (and rtems-ng) osfilesys.c has
{{{
strncpy(LocalPath, VirtualPath, strlen(VirtualPath));
LocalPath[strlen(VirtualPath)] = '\0';
}}} -
ut_os_stubs.c, despite all the UT work recently, still has
{{{
memset(testInfo, 0x00, sizeof(UT_OsTestDesc_t));
strncpy(testInfo->name, testPtr->name, strlen(testPtr->name));
strncpy(testInfo->result, testPtr->result, strlen(testPtr->result));
}}} -
ut_os_stubs.h also has this:
{{{
#define UT_OS_SET_TEST_RESULT_MACRO(VAR1, TSTIDX, TSTNAME, TSTRESULT)
{
memset(&(VAR1.tests[TSTIDX]), 0x00, sizeof(UT_OsTestDesc_t));
strncpy(VAR1.tests[TSTIDX].name, TSTNAME, strlen(TSTNAME));
strncpy(VAR1.tests[TSTIDX].result, TSTRESULT, strlen(TSTRESULT));
(TSTIDX)++;
}
}}}
from osal.
Trac comment by jphickey on 2018-05-14 16:17:14:
Confirmed that these strncpy
issues are still present in the current code. These should be fixed in the next OSAL release.
Some of these exist inside macros -- which is wrong on several levels -- should not be hiding big chunks of code like this inside a macro to begin with. With the UT assert framework this can be much cleaner, but this UT code was never fixed up appropriately.
from osal.
Trac comment by jhageman on 2019-07-03 13:34:40:
Moved open 4.2.2 tickets to 4.3.1
from osal.
Trac comment by jhageman on 2019-07-26 15:13:16:
Milestone renamed
from osal.
Note unit-test-coverage/vxworks6 goes away with #267.
Remaining issues:
unit-tests/shared/ut_os_stubs.h: strncpy(VAR1.name, APINAME, strlen(APINAME)); \
unit-tests/shared/ut_os_stubs.h: strncpy(VAR1.tests[TSTIDX].name, TSTNAME, strlen(TSTNAME)); \
unit-tests/shared/ut_os_stubs.h: strncpy(VAR1.tests[TSTIDX].result, TSTRESULT, strlen(TSTRESULT)); \
unit-tests/shared/ut_os_stubs.c: strncpy(apiInfo->name, apiPtr->name, strlen(apiPtr->name));
unit-tests/shared/ut_os_stubs.c: strncpy(testInfo->name, testPtr->name, strlen(testPtr->name));
unit-tests/shared/ut_os_stubs.c: strncpy(testInfo->result, testPtr->result, strlen(testPtr->result));
from osal.
Fixed in #318 - removed final offenders ut_os_stubs.h and ut_os_stubs.c
from osal.
@dmknutsen can you confirm this is resolved?
from osal.
Yep. In order to identify any remaining issues, I audited all files in the main cFS directory that contain the word strncpy. The attached spreadsheet contains the results of the audit.
No issues remain.
from osal.
Closing as duplicate (issue was resolved by other pull requests.)
from osal.
Related Issues (20)
- Add default build/install scripts for regular and test builds.
- Stack size on POSIX needs to account for internal TCB+TLS
- RTEMS `OS_BinSemTimedWait_Impl` and `OS_CountSemTimedWait_Impl` return incorrect status HOT 3
- OS_FileSysAddFixedMap should use the virtual vs physical path name
- OS_GetErrorName Seg Faults HOT 2
- SemGetInfo partial success cases
- Reinstate RTEMS 4.11 build
- count-sem-test, bin-sem-test intermittently fail on POSIX as non-root user HOT 3
- Add generic RTEMS tar file system support HOT 2
- POSIX implementation does not adhere to user-specified stack pointer
- Softsleep is no longer used
- Update OS_TimedRead and OS_TimedWrite to support sub-millisecond timeout resolution
- Static analysis issues JSC 2.1
- Shared workflow scripts need updates for node.js version 20
- Many compilation warnings when compiling under lp64 data model with -Wconversion option HOT 1
- Permit stack allocation from alternative memory pool on VxWorks
- f-sanitizer detect runtime error in coverage-vxworks-console-testrunner
- f-sanitizer detect runtime error in coverage-shared-idma
- Incorrect function parameter types in os-impl-no-select.c HOT 2
- Unit test for scheduler app requires increase in UT_MAX_FUNC_STUBS value HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from osal.