[CrOS Tether] Fix a buffer overflow issue caught by the ASAN bots.

[CrOS Tether] Fix a buffer overflow issue caught by the ASAN bots.

The issue is that memcpy() was used to copy (N+1) bytes from a source
that was N bytes to a destination that was (N+1) bytes. The fix is only
copying N bytes.

BUG=672263, 734225
Review-Url: https://codereview.chromium.org/2943793002
Cr-Commit-Position: refs/heads/master@{#480235}
Committed: https://chromium.googlesource.com/chromium/src/+/04f2b90690644acc3c81bdd98ab7f379eddb10b7

[Kyle Horimoto, 2017-06-16 22:36:40]

khorimoto@chromium.org changed reviewers:
+ thestig@chromium.org

[Kyle Horimoto, 2017-06-16 22:36:41]

I believe this fix works, but I'm not sure how to test this manually. Lei, can
you help? :)

[Kyle Horimoto, 2017-06-16 22:47:29]

On 2017/06/16 22:36:41, Kyle Horimoto wrote:
> I believe this fix works, but I'm not sure how to test this manually. Lei, can
> you help? :)

Tested it using instructions at
https://www.chromium.org/developers/testing/addresssanitizer, and it passes.
PTAL!

[Lei Zhang, 2017-06-16 22:52:46]

In the CL description:

'\0' character is also called the NUL character or NUL terminator.

It would be nice to wrap commit messages at 72 columns.

[Lei Zhang, 2017-06-16 22:57:37]

https://codereview.chromium.org/2943793002/diff/1/chromeos/components/tether/...
File chromeos/components/tether/ble_scanner.cc (right):

https://codereview.chromium.org/2943793002/diff/1/chromeos/components/tether/...
chromeos/components/tether/ble_scanner.cc:232:
string_contents_ptr[service_data->size()] = '\0';
Do you actually need to NUL-terminate the raw string buffer inside a
std::string?

[Kyle Horimoto, 2017-06-16 22:57:50]

Description was changed from

==========
[CrOS Tether] Fix a buffer overflow issue caught by the ASAN bots.

The issue is that memcpy() was used to copy (N+1) bytes from a source that was N
bytes to a destination that was (N+1) bytes. The fix is only copying N bytes and
manually setting a '\0' character at the end of the string.

BUG=672263,734225
==========

to

==========
[CrOS Tether] Fix a buffer overflow issue caught by the ASAN bots.

The issue is that memcpy() was used to copy (N+1) bytes from a source
that was N bytes to a destination that was (N+1) bytes. The fix is only
copying N bytes and manually setting a NUL terminator at the end of the
string.

BUG=672263,734225
==========

[Kyle Horimoto, 2017-06-16 23:05:04]

>In the CL description:
>
>'\0' character is also called the NUL character or NUL terminator.
>
>It would be nice to wrap commit messages at 72 columns.

Done.

https://codereview.chromium.org/2943793002/diff/1/chromeos/components/tether/...
File chromeos/components/tether/ble_scanner.cc (right):

https://codereview.chromium.org/2943793002/diff/1/chromeos/components/tether/...
chromeos/components/tether/ble_scanner.cc:232:
string_contents_ptr[service_data->size()] = '\0';
On 2017/06/16 22:57:37, Lei Zhang wrote:
> Do you actually need to NUL-terminate the raw string buffer inside a
> std::string?

I checked, and you don't. Removed.

[Kyle Horimoto, 2017-06-16 23:05:20]

Description was changed from

==========
[CrOS Tether] Fix a buffer overflow issue caught by the ASAN bots.

The issue is that memcpy() was used to copy (N+1) bytes from a source
that was N bytes to a destination that was (N+1) bytes. The fix is only
copying N bytes and manually setting a NUL terminator at the end of the
string.

BUG=672263,734225
==========

to

==========
[CrOS Tether] Fix a buffer overflow issue caught by the ASAN bots.

The issue is that memcpy() was used to copy (N+1) bytes from a source
that was N bytes to a destination that was (N+1) bytes. The fix is only
copying N bytes.

BUG=672263,734225
==========

[Lei Zhang, 2017-06-16 23:11:50]

lgtm

[Kyle Horimoto, 2017-06-16 23:12:08]

The CQ bit was checked by khorimoto@chromium.org

[commit-bot: I haz the power, 2017-06-16 23:12:41]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-17 00:13:34]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1497654728367770,
"parent_rev": "9c436a9669b2a3bc5cd10d8d75488715c8bf86aa", "commit_rev":
"04f2b90690644acc3c81bdd98ab7f379eddb10b7"}

[commit-bot: I haz the power, 2017-06-17 00:13:48]

Description was changed from

==========
[CrOS Tether] Fix a buffer overflow issue caught by the ASAN bots.

The issue is that memcpy() was used to copy (N+1) bytes from a source
that was N bytes to a destination that was (N+1) bytes. The fix is only
copying N bytes.

BUG=672263,734225
==========

to

==========
[CrOS Tether] Fix a buffer overflow issue caught by the ASAN bots.

The issue is that memcpy() was used to copy (N+1) bytes from a source
that was N bytes to a destination that was (N+1) bytes. The fix is only
copying N bytes.

BUG=672263,734225

Review-Url: https://codereview.chromium.org/2943793002
Cr-Commit-Position: refs/heads/master@{#480235}
Committed:
https://chromium.googlesource.com/chromium/src/+/04f2b90690644acc3c81bdd98ab7...
==========

[commit-bot: I haz the power, 2017-06-17 00:13:49]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/04f2b90690644acc3c81bdd98ab7...