First, I think calling it a GUI problem was a red herring. The problem
was much more fundamental. I traced it down to a ‘strcmp’. And I am
very put out with whoever used that function in the first place.
Just testing out the solution now and should have a fix pushed shortly.
First, I think calling it a GUI problem was a red herring. The problem
was much more fundamental. I traced it down to a ‘strcmp’. And I am
very put out with whoever used that function in the first place.
Just testing out the solution now and should have a fix pushed shortly.
Tom
There are safe uses for unconstrained string functions. Just, well, not
very many…
–
Marcus L.
Principal Investigator
Shirleys Bay Radio Astronomy Consortium
Also, just pushed a fix. This should take care of things.
Tom
Pardon my ignorance here, but would someone mind explaining this a
little more? My intuition is saying that it’s unsafe to use strcmp on
user input because there’s no checking that there is in fact a sane
string (null terminated), but I haven’t been around long enough to be
sure that’s the issue or if there’s just something more sensible in
boost.
I’d be happy with a link on the topic; I couldn’t find anything useful
googling.
On Fri, Jun 21, 2013 at 12:41 PM, Marcus D. Leech [email protected]
wrote:
There are safe uses for unconstrained string functions. Just, well, not
very many…
–
Marcus L.
No. Never, ever, ever is it ok. I say this using a strlen call now…
(but it’s against strings that are hard-coded into our files by us, so
if that breaks, we have only ourselves to blame).
Also, just pushed a fix. This should take care of things.
On Fri, Jun 21, 2013 at 4:11 PM, Marcus D. Leech [email protected]
wrote:
user input because there’s no checking that there is in fact a sane
be unsafe, and a rich source of buffer-overrun bugs/attacks in the
Shirleys Bay Radio Astronomy Consortium http://www.sbrac.org
Yeah. Basically, you can send a string of any length and strcmp will
attempt to read it, which is what can lead to the buffer overrun.
Another problem, aside from laziness as Marcus says, is that you don’t
often know the correct value for ‘l’. It’s just as suspect to
strlen(a) for some random string a. In this case, I /know/ that string
b is properly terminated (because I wrote it into the code). So I use
the length of b to compare with a and l = strlen(b). There are other
ways, too, but this was the most unobtrusive way to handle this case.
In this case, I /know/ that string
b is properly terminated (because I wrote it into the code). So I use
the length of b to compare with a and l = strlen(b). There are other
ways, too, but this was the most unobtrusive way to handle this case.
Huh … but strcmp will stop comparing at the end of any of the two
strings anyway.
So I don’t see how this: (
)
if (strcmp (name, all[i]->name ()) == 0){
if (strncmp (name, all[i]->name(), strlen(all[i]->name())) == 0){
would provide any more safety again “bad” user strings.
The only difference this code will make is that now “all[i]->name()”
only needs to be a prefix to “name” rather than a full length match.
(which may very well fix the original issue but doesn’t do much about
“unsafe non null terminated strings”)
I’d be happy with a link on the topic; I couldn’t find anything useful googling.
-Nathan
The standard C string functions (the unconstrained ones) are well-known
to be unsafe, and a rich source of buffer-overrun bugs/attacks in the
last couple of decades.
Still, it’s so much more convenient to type strcmp (a,b) rather than
strncmp (a,b,l), etc, etc.
So, sometimes you slip up and use the “unsafe” version.
–
Marcus L.
Principal Investigator
Shirleys Bay Radio Astronomy Consortium
On Sat, Jun 22, 2013 at 4:50 AM, Sylvain M. [email protected]
wrote:
In this case, I /know/ that string
b is properly terminated (because I wrote it into the code). So I use
the length of b to compare with a and l = strlen(b). There are other
ways, too, but this was the most unobtrusive way to handle this case.
Huh … but strcmp will stop comparing at the end of any of the two
strings anyway.
Sylvain,
No, that’s not true. If that were the case then string “abc” would be
equal to string “ab”, and strcmp knows that they are different (I
tested this to be sure; it returns 99, while strcmp(“abc”, “abc”)
returns 0, as it should). I even tested this adding a new line onto
one of the strings and strcmp reads them both through completely and
tests that, not just the length of the shortest string.
Tom
only needs to be a prefix to “name” rather than a full length match.
(which may very well fix the original issue but doesn’t do much about
“unsafe non null terminated strings”)
Oh yeah, you’re right about that. It’s reading in ‘name’ in the first
place without constraints that would cause the problem with non null
terminated strings. But it does fix a problem we had.
Huh … but strcmp will stop comparing at the end of any of the two
strings anyway.
No, that’s not true. If that were the case then string “abc” would be
equal to string “ab”, and strcmp knows that they are different (I
tested this to be sure; it returns 99, while strcmp(“abc”, “abc”)
returns 0, as it should). I even tested this adding a new line onto
one of the strings and strcmp reads them both through completely and
tests that, not just the length of the shortest string.
Sorry I wasn’t clear.
I didn’t mean that it will return a match. I meant that it will stop
scanning through the strings as soon as one ends.
Whatever the result of the last comparaison it did will be used to
determine the return value.
In the case of “ab” vs “abc” the last comparaison is between ‘\0’ and
‘c’.
On Sat, Jun 22, 2013 at 8:48 AM, Sylvain M. [email protected]
wrote:
tests that, not just the length of the shortest string.
Sylvain
Ok, I see your point now. The problem was arising in that the file
that we write to .gnuradio/prefs is just straight text, and we read in
the whole file. When we write it, there’s no new line. But some
systems (those that were having some of the issues) were adding an
extra newline after reading it. Strange, and wrong, and I’m not sure
what was doing it. But that new line was making the comparison fail.
Using strncmp allows us to ignore that by only checking the length of
the string we’re actually interested in. (An alternative would be to
only read in the first line of the file and remove any newline
characters.)
Tom
This forum is not affiliated to the Ruby language, Ruby on Rails framework, nor any Ruby applications discussed here.