MantisBT - Doomseeker
View Issue Details
0003498Doomseeker[All Projects] Securitypublic2018-09-19 13:292019-07-30 10:13
WubTheCaptain 
Pol M 
lowtweakalways
closedfixed 
amd64OpenBSD6.4-current
1.1 
1.31.3 
0003498: GCC compiler warns of unsafe C/C++ string functions used instead of safer alternatives
During source compile on OpenBSD, the gcc compiler prints warnings about use of unsafe functions and suggests to replace them (irregardless of if their use is safe or unsafe in this scenario).

[ 83%] Linking CXX executable ../../doomseeker
CMakeFiles/doomseeker.dir/scanner.cpp.o: In function `Scanner::scriptMessage(Scanner::MessageLevel, char 
const*, ...) const':
scanner.cpp:(.text+0x3b63): warning: sprintf() is often misused, please use snprintf()
/usr/local/lib/qt5/./libQt5Core.so.2.2: warning: rand_r() is not random, it is deterministic.
CMakeFiles/doomseeker.dir/random.cpp.o: In function `Random::nextUShort(unsigned short)':
random.cpp:(.text+0x14f): warning: rand() may return deterministic values, is that what you want?
/usr/X11R6/lib/libGL.so.17.1: warning: random() may return deterministic values, is that what you want?

/usr/local/lib/libglib-2.0.so.4201.0: warning: stpcpy() is dangerous; do not use it
/usr/local/lib/libglib-2.0.so.4201.0: warning: strcpy() is almost always misused, please use strlcpy()

/usr/local/lib/libglib-2.0.so.4201.0: warning: vsprintf() is often misused, please use vsnprintf()
/usr/local/lib/libglib-2.0.so.4201.0: warning: strcat() is almost always misused, please use strlcat()
gcc 4.2.1 on OpenBSD (with gcc-local(1) patches). Something like this:

pkg_add cmake mercurial qt5 # as superuser privileges
mkdir -p $HOME/.local/src/ && cd $HOME/.local/src/
hg clone'https://bitbucket.org/Doomseeker/doomseeker/ [^]'
mkdir -p /tmp/doomseeker && cd /tmp/doomseeker
Qt5Widgets_DIR=/usr/local/lib/qt5/cmake/Qt5Widgets/
Qt5LinguistTools_DIR=/usr/local/lib/qt5/cmake/Qt5LinguistTools/
Qt5Multimedia_DIR=/usr/local/lib/qt5/cmake/Qt5Multimedia
Qt5Xml_DIR=/usr/local/lib/qt5/cmake/Qt5Xml
export Qt5Widgets_DIR Qt5LinguistTools_DIR Qt5Multimedia_DIR Qt5Xml_DIR
export CPATH=$CPATH:/usr/local/include
cmake $HOME/.local/src/doomseeker/
CC=gcc CXX=g++ make -j4
The following files create warnings in Doomseeker:

  • src/core/scanner.cpp
  • src/core/random.cpp
No tags attached.
child of 0003499assigned Pol M Port Doomseeker to OpenBSD 
Issue History
2018-09-19 13:29WubTheCaptainNew Issue
2018-09-19 13:29WubTheCaptainOS => OpenBSD
2018-09-19 13:29WubTheCaptainOS Version => 6.4-current
2018-09-19 13:29WubTheCaptainPlatform => amd64
2018-09-19 17:49WubTheCaptainNote Added: 0019575
2018-09-22 01:21WubTheCaptainNote Added: 0019607
2018-09-29 15:07WubTheCaptainSeverityminor => tweak
2018-10-05 06:44WubTheCaptainTarget Version => 1.2
2018-10-09 14:27WubTheCaptainTarget Version1.2 =>
2018-12-17 05:21WubTheCaptainCategoryBug => Security
2018-12-17 08:49FilysteaNote Added: 0020267
2018-12-20 17:33FilysteaNote Added: 0020276
2018-12-21 06:54WubTheCaptainPrioritynormal => low
2018-12-25 18:27FilysteaNote Added: 0020282
2018-12-25 18:28FilysteaNote Edited: 0020282bug_revision_view_page.php?bugnote_id=20282#r12332
2018-12-25 18:28FilysteaNote Edited: 0020282bug_revision_view_page.php?bugnote_id=20282#r12333
2019-01-06 06:31WubTheCaptainAssigned To => WubTheCaptain
2019-01-06 06:31WubTheCaptainStatusnew => acknowledged
2019-05-21 22:51Pol MAssigned ToWubTheCaptain => Pol M
2019-05-21 22:51Pol MStatusacknowledged => assigned
2019-05-21 22:52Pol MRelationship addedchild of 0003499
2019-05-26 14:04Pol MNote Added: 0020693
2019-05-28 16:30Pol MNote Added: 0020702
2019-05-28 16:30Pol MStatusassigned => needs testing
2019-05-28 16:31Pol MNote Edited: 0020702bug_revision_view_page.php?bugnote_id=20702#r12602
2019-05-30 06:12WubTheCaptainTarget Version => 1.3
2019-07-20 01:15Pol MNote Added: 0020896
2019-07-20 01:16Pol MStatusneeds testing => resolved
2019-07-20 01:16Pol MFixed in Version => 1.3
2019-07-20 01:16Pol MResolutionopen => fixed
2019-07-30 10:13WubTheCaptainStatusresolved => closed

Notes
(0019575)
WubTheCaptain   
2018-09-19 17:49   
Might've been clang, actually. Anyway...
(0019607)
WubTheCaptain   
2018-09-22 01:21   
Also tools/updaterevision/updaterevision.c's main function, strcpy().
(0020267)
Filystea   
2018-12-17 08:49   
clang is default openbsd compiler.

Glad there are people using my fav sys ;-)

Btw.

This is not that important. The problem is:

You still can only compile servers because of fmod crap.
Most times you will be dealing with malloc.h changing to stdlib.h;-)

I had a rage topic about it once. Did compile for sake of compiling but never used.

openbsd for life <3
(0020276)
Filystea   
2018-12-20 17:33   
I belive I was not clear enough. Lock this up. those warnings are just PROPOSITIONS.

Seriusly. I write quite a bit of C and this is silly.
strlxxx is not standard. Ofc strcpy can do damage if programer fucks up but the function is not bad. Even gets is not bad ( yeah - I just said it ).
Those are just crappy clang warnings.

And openbsd did not pick clang because it's somehow *more secure makes more secure code or what ever *.
(0020282)
Filystea   
2018-12-25 18:27   
(edited on: 2018-12-25 18:28)
Actually it kind of bugged me. Is that really clang so I went on #openbsd free-node and seems this is openbsd addon for linker.

Anyway strlcpy is lame to use.
Use strncpy strnlen etc. Just pass buff_size - 1 and have the last byte set to 0/nul. Using 'n' family also fixes the warning. If anyone cares. hue hue ;-)

(0020693)
Pol M   
2019-05-26 14:04   
This is warned exclusively by OpenBSD, regardless of the compiler.
srand() is called previously to rand(), so for the exclusive usage of creating random strings it would be more than OK. That said, since we are using C++ 11, I'll change it to use the std::random library, which is better and provides lots of options. For reference, I'm not using QRandomGenerator because it was introduced in 5.10, and windows builds may not work.

strcpy() is always used in safe situations, there is nothing to change.
src/core/main.cpp:724
tools/updaterevision/updaterevision.c:114
sprintf() is also OK, so I also see no point on changing it.
tools/updateinstaller/src/UpdateDialogAscii.cpp:698
(0020702)
Pol M   
2019-05-28 16:30   
(edited on: 2019-05-28 16:31)
The needed change on the random class is done

(0020896)
Pol M   
2019-07-20 01:15   
Resolving since there is not much to check. Code works ¯\_(ツ)_/¯