search

mp49 / parker6k

EPICS support for the Parker 6K controller
1 stars 2 forks source link

potential buffer overrun #3

Closed mark0n closed 9 years ago

mark0n commented 9 years ago

Have a look at parker6kController.cpp:350:

memset(response, 0, sizeof(response));

response is a pointer - so this line is probably not doing what you intended. Did you mean

memset(response, 0, strlen(response));
mp49 commented 9 years ago

Hi Martin,

I've getting a completely clean compile, so we must be using different compilers, is yours GCC 4.8 or above?

Mine is: [mkp@bl99-dassrv1 parker6k]$ gcc -v Using built-in specs. Target: x86_64-redhat-linux Configured with: ../configure --prefix=/usr --mandir=/usr/share/man --infodir=/usr/share/info --with-bugurl=http://bugzilla.redhat.com/bugzilla --enable-bootstrap --enable-shared --enable-threads=posix --enable-checking=release --with-system-zlib --enable-__cxa_atexit --disable-libunwind-exceptions --enable-gnu-unique-object --enable-languages=c,c++,objc,obj-c++,java,fortran,ada --enable-java-awt=gtk --disable-dssi --with-java-home=/usr/lib/jvm/java-1.5.0-gcj-1.5.0.0/jre --enable-libgcj-multifile --enable-java-maintainer-mode --with-ecj-jar=/usr/share/java/eclipse-ecj.jar --disable-libjava-multilib --with-ppl --with-cloog --with-tune=generic --with-arch_32=i686 --build=x86_64-redhat-linux Thread model: posix gcc version 4.4.7 20120313 (Red Hat 4.4.7-3) (GCC)

(a standard CentOS 6.4 build, which I guess is getting older and older now).

Must be a new error you're seeing, because doing a sizeof() on a char array pointer is usually ok. But if using strlen fixes it, then that's ok.

But there were a load of additional errors. I'll take a look tomorrow. I may have to build myself a new VM to look into all of them.

Cheers, Matt

On Nov 4, 2014, at 4:38 PM, Martin Konrad notifications@github.com wrote:

Have a look at parker6kController.cpp:350:

memset(response, 0, sizeof(response));

response is a pointer - so this line is probably not doing what you intended. Did you mean

memset(response, 0, strlen(response));

— Reply to this email directly or view it on GitHub.

mark0n commented 9 years ago

I'm using GCC 4.9.1 on the brand new Ubuntu 14.10 ("utopic unicorn").

mark0n commented 9 years ago

Doing a sizeof on a char array pointer might work (in this case the sizeof operator returns the size of the array). BUT: We do not dealing with an array here but only with a pointer to a char (which might be the first element of a C string or not). So there is no way to determine the size of this array. What you'll get instead is the size of the pointer (8 bytes on most 64 bit machines). Depending on the length of the C string it might not be large enough to hold 8 bytes or might not get cleared out completely.

Is there a reason why you aren't using C++ strings as parameters for p6kController::errorResponse?

mp49 commented 9 years ago

I've fixed the sizeof issue. strlen was the correct one to use. Committed to the master branch.

I think I used char pointers to match what the strstr function required as input. When I was at Diamond I tended to avoid C++ strings because our VxWorks BSP didn't support them.