Use base::span rather than pointers in AuthenticatorQRCode

This provides some bounds checks and avoids a potential mishap if
OnPaint were ever called twice in a row somehow. Also spans are fun.

Bug: none
Change-Id: Ibc6043099c99e41f67f3f3a2132b01766b8353dd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1766252
Commit-Queue: David Benjamin <davidben@chromium.org>
Commit-Queue: Adam Langley <agl@chromium.org>
Auto-Submit: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@chromium.org>
Cr-Commit-Position: refs/heads/master@{#689710}

chrome/browser/ui/views/webauthn/authenticator_qr_code.cc

@@ -17,7 +17,8 @@ static_assert(AuthenticatorQRCode::kNumSegments != 0 &&
 // Generate generates a QR code containing the given data and returns a
 // pointer to an array of kSize×kSize bytes where the least-significant bit of
 // each byte is set if that tile should be "black".
-const uint8_t* AuthenticatorQRCode::Generate(const uint8_t in[kInputBytes]) {
+base::span<const uint8_t, AuthenticatorQRCode::kTotalSize>
+AuthenticatorQRCode::Generate(const uint8_t in[kInputBytes]) {
   memset(d_, 0, sizeof(d_));
   PutVerticalTiming(6);
   PutHorizontalTiming(6);

chrome/browser/ui/views/webauthn/authenticator_qr_code.h

@@ -8,6 +8,8 @@
 #include <stddef.h>
 #include <stdint.h>
 
+#include "base/containers/span.h"
+
 // AuthenticatorQRCode generates version three, class Q QR codes that carry 32
 // bytes of raw data. References in the following comments refer to ISO 18004
 // (3rd edition).
@@ -17,6 +19,9 @@ class AuthenticatorQRCode {
   // table 1. (The colored squares in in QR codes are called tiles in the
   // spec.)
   static constexpr int kSize = 29;
+  // kTotalSize is the total number of tiles for a v3 QR code, in both
+  // directions.
+  static constexpr int kTotalSize = kSize * kSize;
   // These values are taken from table 9 (page 38) for a version three, class Q
   // QR code.
   static constexpr size_t kTotalBytes = 70;
@@ -30,9 +35,9 @@ class AuthenticatorQRCode {
   static constexpr size_t kInputBytes = kDataBytes - 2;
 
   // Generate generates a QR code containing the given data and returns a
-  // pointer to an array of kSize×kSize bytes where the least-significant bit of
+  // pointer to an array of kTotalSize bytes where the least-significant bit of
   // each byte is set if that tile should be "black".
-  const uint8_t* Generate(const uint8_t in[kInputBytes]);
+  base::span<const uint8_t, kTotalSize> Generate(const uint8_t in[kInputBytes]);
 
  private:
   // MaskFunction3 implements one of the data-masking functions. See figure 21.

chrome/browser/ui/views/webauthn/authenticator_qr_code_test.cc

@@ -15,11 +15,12 @@ TEST(AuthenticatorQRCode, Generate) {
   AuthenticatorQRCode qr;
   uint8_t input[AuthenticatorQRCode::kInputBytes];
   memset(input, 'a', sizeof(input));
-  const uint8_t* qr_data = qr.Generate(input);
+  auto qr_data = qr.Generate(input);
 
+  int index = 0;
   for (int y = 0; y < AuthenticatorQRCode::kSize; y++) {
     for (int x = 0; x < AuthenticatorQRCode::kSize; x++) {
-      ASSERT_EQ(0, *(qr_data++) & 0b11111100);
+      ASSERT_EQ(0, qr_data[index++] & 0b11111100);
     }
   }
 }

chrome/browser/ui/views/webauthn/authenticator_qr_sheet_view.cc

@@ -85,9 +85,8 @@ class QRView : public views::View {
   // kDinoY is the y-coordinate of the dino image.
   static constexpr int kDinoY = kMid - (kDinoHeight * kDinoTilePixels) / 2;
 
-  explicit QRView(const uint8_t qr_data[QRCode::kInputBytes]) {
-    qr_tiles_ = qr_.Generate(qr_data);
-  }
+  explicit QRView(const uint8_t qr_data[QRCode::kInputBytes])
+      : qr_tiles_(qr_.Generate(qr_data)) {}
   ~QRView() override {}
 
   void RefreshQRCode(const uint8_t new_qr_data[QRCode::kInputBytes]) {
@@ -151,9 +150,10 @@ class QRView : public views::View {
         off);
 
     // Paint the QR code.
+    int index = 0;
     for (int y = 0; y < QRCode::kSize; y++) {
       for (int x = 0; x < QRCode::kSize; x++) {
-        SkColor tile_color = (*qr_tiles_++) & 1 ? on : off;
+        SkColor tile_color = qr_tiles_[index++] & 1 ? on : off;
         canvas->FillRect(gfx::Rect((x + 2) * kTilePixels, (y + 2) * kTilePixels,
                                    kTilePixels, kTilePixels),
                          tile_color);
@@ -197,7 +197,7 @@ class QRView : public views::View {
   }
 
   QRCode qr_;
-  const uint8_t* qr_tiles_ = nullptr;
+  base::span<const uint8_t, QRCode::kTotalSize> qr_tiles_;
   unsigned state_ = 0;
 
   DISALLOW_COPY_AND_ASSIGN(QRView);