C client does not check the version compatibility correctly

[GoogleCodeExporter]

What steps will reproduce the problem?
1. In debug mode, comparison of client and server version number return Warning
2.
3.

What is the expected output? What do you see instead?
Don't return Warning if version number is the same

Since  revision r829, server add to version number his type (-web100 or 
-Web10G).
Comparison which is wrong is at line: 
https://code.google.com/p/ndt/source/browse/trunk/src/web100clt.c#769

Original issue reported on code.google.com by smale...@soldevelo.com on 7 Feb 2014 at 8:42

[GoogleCodeExporter]

Original comment by smale...@soldevelo.com on 7 Feb 2014 at 9:29

• Changed state: Started

[GoogleCodeExporter]

Fix issue with incorrect check version compatibility i C client. Changes are 
available on applet_109 branch. Feel free to review it.

Original comment by smale...@soldevelo.com on 7 Feb 2014 at 12:48

• Changed state: Complete

[GoogleCodeExporter]

Seems fine to me. Merge away.

Original comment by AaronMat...@gmail.com on 7 Feb 2014 at 1:53

[GoogleCodeExporter]

I see a couple of issues with this,

The main issue is that strncpy() doesn't null terminate the string if the 
string is longer than the allowed size. This means the comparison is actually 
being made on random memory and will be incorrect and could cause a segfault.

While unlikely, a check to ensure that 'buff' is long enough needs to be done 
before &buff[strlen(buff) - 6] otherwise this could result in a negative index 
resulting in similar issues.

Also "web100" should be have a capital 'w' i.e. should be "Web100" in the 
comparison see the definition TCP_STAT_NAME here 
https://code.google.com/p/ndt/source/browse/trunk/src/web100srv.h#299.

This could be rewritten as following to avoid strncpy() and the other issues as 
well as duplicated code. I've tested this against the following servers 
3.6.6-rc1-Web10G, 3.6.6-rc1-Web100 and 3.6.5.2.

  ServerType = "Web100";
  if (strlen(buff) > 8) { // 7 (+ 1 for the 'v' at the start)
    // Since the addition of Web10G the server will append -Web10X
    // We remove this before comparing versions
    if (strcmp(&buff[strlen(buff) - 7], "-Web10G") == 0) {
      ServerType = "Web10G";
      buff[strlen(buff) - 7] = '\0';
    }
    if (strcmp(&buff[strlen(buff) - 7], "-Web100") == 0) {
      buff[strlen(buff) - 7] = '\0';
    }
  }

  if (strcmp(&buff[1], VERSION)) {
    log_println(1, "WARNING: NDT server has different version number (%s)",
                &buff[1]);
  }

Original comment by rsanger...@gmail.com on 8 Feb 2014 at 1:38

[GoogleCodeExporter]

Thanks, for yours suggestions. You're right. On branch applet_109 is available 
new version. Now it should be OK.

Original comment by smale...@soldevelo.com on 10 Feb 2014 at 8:01

[GoogleCodeExporter]

Cool, looks good merge it into trunk.

Original comment by rsanger...@gmail.com on 10 Feb 2014 at 9:57

[GoogleCodeExporter]

Original comment by smale...@soldevelo.com on 10 Feb 2014 at 12:15

• Changed state: Fixed

[GoogleCodeExporter]

Original comment by skost...@soldevelo.com on 23 Jun 2014 at 5:53

• Added labels: Milestone-NDT-3.7.0