Code Review Before Version 1.4.0 is Tagged

.travis.yml

@@ -2,6 +2,7 @@ language: php
 
 php:
 - 7.0
+- 7.1
 
 env:
 - DB=pgsql

src/Airship.php

@@ -118,31 +118,31 @@ function autoload(string $namespace, string $directory): bool
     $dir = preg_replace('#^~'.$ds.'#', ROOT.$ds, $directory);
 
     return \spl_autoload_register(
-        function(string $class) use ($ds, $ns, $dir)
+        function (string $class) use ($ds, $ns, $dir)
         {
             // project-specific namespace prefix
-            $prefix = $ns.'\\';
+            $prefix = $ns . '\\';
 
             // base directory for the namespace prefix
-            $base_dir =  $dir.$ds;
+            $base_dir =  $dir . $ds;
 
             // does the class use the namespace prefix?
-            $len = strlen($prefix);
-            if (strncmp($prefix, $class, $len) !== 0) {
+            $len = Binary::safeStrlen($prefix);
+            if (\strncmp($prefix, $class, $len) !== 0) {
                 // no, move to the next registered autoloader
                 return;
             }
 
             // get the relative class name
-            $relative_class = substr($class, $len);
+            $relative_class = Binary::safeSubstr($class, $len);
 
             // replace the namespace prefix with the base directory, replace
             // namespace separators with directory separators in the relative 
             // class name, append with .php
-            $file = $base_dir . str_replace('\\', $ds, $relative_class) . '.php';
+            $file = $base_dir . \str_replace('\\', $ds, $relative_class) . '.php';
 
             // if the file exists, require it
-            if (file_exists($file)) {
+            if (\file_exists($file)) {
                 require $file;
             }
         }
@@ -653,13 +653,13 @@ function queryString(
         \Sodium\CRYPTO_GENERICHASH_BYTES_MIN
     );
     if (empty($_cache[$cacheKey])) {
-        $driver = preg_replace('/[^a-z]/', '', \strtolower($driver));
+        $driver = \preg_replace('/[^a-z]/', '', \strtolower($driver));
         $path = !empty($cabin)
             ? ROOT . '/Cabin/' . $cabin.'/Queries/' . $driver . '.json'
             : ROOT . '/Engine/Queries/' . $driver . '.json';
         $_cache[$cacheKey] = \Airship\loadJSON($path);
     }
-    $split_key = explode('.', $index);
+    $split_key = \explode('.', $index);
     $v = $_cache[$cacheKey];
     foreach ($split_key as $k) {
         if (!\array_key_exists($k, $v)) {

src/Cabin/Bridge/Lens/cargo/bridge_authors_list.twig

@@ -81,31 +81,31 @@
     {% for auth in authors %}
         <div class="table-row">
             <div class="table-cell table-min-width">
-                <a{#
-                    #} href="{{ cabin_url() }}author/edit/{{ auth.authorid|e('html_attr') }}"{#
-                    #} class="pure-button pure-button-secondary icon-1px-right"{#
-                    #} title="{{ __("Edit Author Profile")|e('html_attr') }}"{#
-                #}><i class="fa fa-pencil"></i></a>{#
-                #}<a{#
-                    #} href="{{ cabin_url() }}author/users/{{ auth.authorid|e('html_attr') }}"{#
-                    #} class="pure-button pure-button-primary icon-1px-right"{#
-                    #} title="{{ __("Manage Members")|e('html_attr') }}"{#
-                #}><i class="fa fa-users"></i></a>{#
-                #}<a{#
-                    #} href="{{ cabin_url() }}author/photos/{{ auth.authorid|e('html_attr') }}"{#
-                    #} class="pure-button pure-button-secondary icon-1px-right"{#
-                    #} title="{{ __("Author Photos")|e('html_attr') }}"{#
-                #}><i class="fa fa-photo"></i></a>{#
-                #}<a{#
-                    #} href="{{ cabin_url() }}author/files/{{ auth.authorid|e('html_attr') }}/Hull"{#
-                    #} class="pure-button pure-button-primary icon-1px-right"{#
-                    #} title="{{ __("Author Files")|e('html_attr') }}"{#
-                #}><i class="fa fa-folder-open-o"></i></a>{#
-                #}<a{#
-                    #} href="{{ cabin_url() }}author/delete/{{ auth.authorid|e('html_attr') }}"{#
-                    #} class="pure-button pure-button-secondary"{#
-                    #} title="{{ __("Delete Author")|e('html_attr') }}"{#
-                #}><i class="fa fa-trash-o"></i></a>
+                <a
+                    href="{{ cabin_url() }}author/edit/{{ auth.authorid|e('html_attr') }}"
+                    class="pure-button pure-button-secondary icon-1px-right"
+                    title="{{ __("Edit Author Profile")|e('html_attr') }}"
+                ><i class="fa fa-pencil"></i></a>{#
+                #}<a
+                    href="{{ cabin_url() }}author/users/{{ auth.authorid|e('html_attr') }}"
+                    class="pure-button pure-button-primary icon-1px-right"
+                    title="{{ __("Manage Members")|e('html_attr') }}"
+                ><i class="fa fa-users"></i></a>{#
+                #}<a
+                    href="{{ cabin_url() }}author/photos/{{ auth.authorid|e('html_attr') }}"
+                    class="pure-button pure-button-secondary icon-1px-right"
+                    title="{{ __("Author Photos")|e('html_attr') }}"
+                ><i class="fa fa-photo"></i></a>{#
+                #}<a
+                    href="{{ cabin_url() }}author/files/{{ auth.authorid|e('html_attr') }}/Hull"
+                    class="pure-button pure-button-primary icon-1px-right"
+                    title="{{ __("Author Files")|e('html_attr') }}"
+                ><i class="fa fa-folder-open-o"></i></a>{#
+                #}<a
+                    href="{{ cabin_url() }}author/delete/{{ auth.authorid|e('html_attr') }}"
+                    class="pure-button pure-button-secondary"
+                    title="{{ __("Delete Author")|e('html_attr') }}"
+                ><i class="fa fa-trash-o"></i></a>
             </div>
             <div class="table-cell">
                 {{ auth.name }}

src/Cabin/Bridge/Lens/cargo/bridge_blog_post_history_view.twig

@@ -44,34 +44,34 @@
     <div class="table-row">
         <label class="table-cell" for="blog_post_slug">{{ __("Slug") }}:</label>
         <div class="table-cell bottom-pad">
-            <input{#
-                #} class="full-width"{#
-                #} disabled="disabled"{#
-                #} readonly="readonly"{#
-                #} id="blog_post_slug"{#
-                #} type="text"{#
-                #} name="slug"{#
-                #} data-original="{{ blogpost.slug|e('html_attr') }}"{#
-                #} value="{{ blogpost.slug|e('html_attr') }}"{#
-                #} pattern="^[a-z0-9][a-z0-9\-]+$"{#
-            #} />
+            <input
+                 class="full-width"
+                 disabled="disabled"
+                 readonly="readonly"
+                 id="blog_post_slug"
+                 type="text"
+                 name="slug"
+                 data-original="{{ blogpost.slug|e('html_attr') }}"
+                 value="{{ blogpost.slug|e('html_attr') }}"
+                 pattern="^[a-z0-9][a-z0-9\-]+$"
+             />
         </div>
     </div>
 
     {% if can_publish %}
         <div class="table-row">
             <label class="table-cell table-min-width" for="blog_post_publish_date">{{ __("Date/Time Published") }}:</label>
             <div class="table-cell bottom-pad">
-                <input{#
-                    #} class="full-width"{#
-                    #} disabled="disabled"{#
-                    #} readonly="readonly"{#
-                    #} id="blog_post_published"{#
-                    #} type="text"{#
-                    #} name="published"{#
-                    #} value="{{ blogpost.published|e('html_attr') }}"{#
-                    #} pattern="^2[0-9]{3}\-(0[0-9]|1[0-2])\-([012][0-9]|3[0-1])(T| )([0-1][0-9]|2[0-3]):[0-5][0-9]:[0-5][0-9]([\.:\+0-9]+)?$"{#
-                #} />
+                <input
+                     class="full-width"
+                     disabled="disabled"
+                     readonly="readonly"
+                     id="blog_post_published"
+                     type="text"
+                     name="published"
+                     value="{{ blogpost.published|e('html_attr') }}"
+                     pattern="^2[0-9]{3}\-(0[0-9]|1[0-2])\-([012][0-9]|3[0-1])(T| )([0-1][0-9]|2[0-3]):[0-5][0-9]:[0-5][0-9]([\.:\+0-9]+)?$"
+                 />
             </div>
         </div>
     {% endif %}
@@ -112,11 +112,11 @@
                 >
                     <option value="">{% if authors %} -- {{ __("Please Select an Author") }} -- {% endif %}</option>
                     {% for auth in authors %}
-                        <option{#
-                            #} value="{{ auth.authorid }}"{#
-                            #}{% if auth.authorid == blogpost.author %}{#
-                                #} selected="selected"{#
-                            #}{% endif %}>{{ auth.name }}</option>
+                        <option
+                             value="{{ auth.authorid }}"
+                            {% if auth.authorid == blogpost.author %}
+                                 selected="selected"
+                            {% endif %}>{{ auth.name }}</option>
                     {% endfor %}
                 </select>
             {% else %}

src/Cabin/Bridge/Lens/cargo/rich_text_editor.twig

@@ -1 +1 @@
-{% include "common/motif_extra/rich_text_editor.twig" %}
+{% include "common/motif_extra/rich_text_editor.twig" %}

src/Cabin/Bridge/Lens/motif_configure.twig

@@ -9,4 +9,4 @@
     <script src="{{ "/js/wysihtml5/parser_rules/advanced_and_extended.js"|cachebust }}"></script>
     <script src="{{ "/js/colors.js"|cachebust }}"></script>
     <script src="{{ "/static/Bridge/editor.js"|cachebust }}"></script>
-{% endblock %}
+{% endblock %}

src/Cabin/Hull/Lens/cargo/footer.twig

@@ -15,4 +15,4 @@
             #}Paragon Initiative Enterprises{#
         #}</a>
     </p>
-{% endif %}
+{% endif %}

src/Engine/AutoPilot.php

@@ -8,6 +8,7 @@
     FallbackLoop
 };
 use Airship\Engine\Contract\RouterInterface;
+use ParagonIE\ConstantTime\Binary;
 use ParagonIE\CSPBuilder\CSPBuilder;
 
 /**
@@ -94,11 +95,11 @@ public function setActiveCabin(array $cabin, string $prefix)
         if ($prefix === '*') {
             self::$patternPrefix = '';
         } elseif ($prefix[0] === '*') {
-            self::$patternPrefix = \substr($prefix, 2);
+            self::$patternPrefix = Binary::safeSubstr($prefix, 2);
         } else {
             $start = \strpos($prefix, '/');
             if ($start !== false) {
-                self::$patternPrefix = \substr($prefix, $start + 1);
+                self::$patternPrefix = Binary::safeSubstr($prefix, $start + 1);
             }
         }
     }
@@ -208,7 +209,7 @@ public static function isActiveCabinKey(
         if ($cabinKey[0] === '*') {
             if ($cabinKey[1] === '/') {
                 // */some_dir/
-                $pattern = \preg_quote(\substr($cabinKey, 2), '#');
+                $pattern = \preg_quote(Binary::safeSubstr($cabinKey, 2), '#');
                 if (\preg_match('#^/'.$pattern.'#', $uri) === 1) {
                     return $https_only
                         ? self::forceHTTPS()
@@ -225,11 +226,11 @@ public static function isActiveCabinKey(
                     ? self::forceHTTPS($scheme)
                     : true;
             } elseif ($pos !== false) {
-                $sub = \substr($cabinKey, $pos);
-                $host = \substr($cabinKey, 0, $pos);
+                $sub = Binary::safeSubstr($cabinKey, $pos);
+                $host = Binary::safeSubstr($cabinKey, 0, $pos);
                 if (
                     \strtolower($activeHost) === \strtolower($host) &&
-                    \preg_match('#^'.\preg_quote($sub, '#').'#', $uri)
+                    \preg_match('#^' . \preg_quote($sub, '#') . '#', $uri)
                 ) {
                     return $https_only
                         ? self::forceHTTPS($scheme)
@@ -257,7 +258,7 @@ public function route()
                 self::$mypath = $path;
                 self::$path = \substr(
                     $_SERVER['REQUEST_URI'],
-                    \strlen(self::$patternPrefix) + 1
+                    Binary::safeStrlen(self::$patternPrefix) + 1
                 );
                 try {
                     // Attempt to serve the page:

src/Engine/Bolt/Get.php

@@ -2,6 +2,8 @@
 declare(strict_types=1);
 namespace Airship\Engine\Bolt;
 
+use ParagonIE\ConstantTime\Binary;
+
 /**
  * Trait Get
  *
@@ -27,9 +29,9 @@ protected function httpGetParams(string &$lastPiece = null): array
         $p = \strpos($lastPiece, '?');
         if ($p !== false && empty($_GET)) {
             $_GET = \Airship\array_from_http_query(
-                \substr($lastPiece, $p + 1)
+                Binary::safeSubstr($lastPiece, $p + 1)
             );
-            $lastPiece = \substr($lastPiece, 0, $p);
+            $lastPiece = Binary::safeSubstr($lastPiece, 0, $p);
         }
         return $_GET;
     }

src/Engine/Bolt/Security.php

@@ -72,7 +72,7 @@ public function tightenSecurityBolt()
      * @param string $context context regex (in perm_contexts)
      * @param string $cabin (defaults to current cabin)
      * @param integer $userID (defaults to current user)
-     * @return boolean
+     * @return bool
      */
     public function can(
         string $action,
@@ -110,6 +110,7 @@ public function getActiveUserId(): int
 
     /**
      * Are we currently logged in as an admin?
+     *
      * @param integer $userId (defaults to current user)
      * @return bool
      */
@@ -181,6 +182,8 @@ protected function doAutoLogin(
         $state = State::instance();
         try {
             $userId = $this->airship_auth->loginByToken($token);
+            \Sodium\memzero($token);
+
             if (!$this->verifySessionCanary($userId, false)) {
                 return false;
             }
@@ -288,7 +291,7 @@ public function verifySessionCanary(int $userID, bool $logOut = true): bool
         );
         if (empty($canary)) {
             $this->log(
-                'What is this even.',
+                'No session canary was registered with this user in the database.',
                 LogLevel::DEBUG,
                 [
                     'database' => $canary,

src/Engine/Bolt/Supplier.php

@@ -9,6 +9,7 @@
     FileSystem\FileNotFound
 };
 use Airship\Engine\Continuum\Supplier as SupplierObject;
+use ParagonIE\ConstantTime\Binary;
 
 /**
  * Trait Supplier
@@ -31,7 +32,7 @@ trait Supplier
      */
     public function createSupplier(string $channelName, array $data): SupplierObject
     {
-        $supplierName = $data['supplier'];
+        $supplierName = $this->escapeSupplierName($data['supplier']);
         if (\file_exists(ROOT . '/config/supplier_keys/' . $supplierName . '.json')) {
             throw new CouldNotCreateSupplier(
                 'File already exists: config/supplier_keys/' . $supplierName . '.json'
@@ -58,6 +59,19 @@ public function createSupplier(string $channelName, array $data): SupplierObject
         return $this->getSupplier($supplierName, true);
     }
 
+    /**
+     * @param string $supplier
+     * @return string
+     */
+    public function escapeSupplierName(string $supplier): string
+    {
+        return \preg_replace(
+            '#[^A-Za-z0-9\-\_]#',
+            '',
+            $supplier
+        );
+    }
+
     /**
      * For objects that don't bother caching
      *
@@ -69,6 +83,7 @@ public function createSupplier(string $channelName, array $data): SupplierObject
      */
     public function getSupplierDontCache(string $supplier): SupplierObject
     {
+        $supplier = $this->escapeSupplierName($supplier);
         if (!\file_exists(ROOT . '/config/supplier_keys/' . $supplier . '.json')) {
             throw new NoSupplier(
                 \__(
@@ -103,7 +118,13 @@ public function getSupplier(
                 );
                 foreach ($allSuppliers as $supplierKeyFile) {
                     // We want everything except the .json
-                    $supplier = \substr($this->getEndPiece($supplierKeyFile), 0, -5);
+                    $supplier = $this->escapeSupplierName(
+                        Binary::safeSubstr(
+                            $this->getEndPiece($supplierKeyFile),
+                            0,
+                            -5
+                        )
+                    );
                     try {
                         $data = \Airship\loadJSON($supplierKeyFile);
                     } catch (FileNotFound $ex) {
@@ -118,6 +139,7 @@ public function getSupplier(
         // Otherwise, we're just fetching one supplier's keys
         if ($force_flush || empty($this->supplierCache[$supplier])) {
             try {
+                $supplier = $this->escapeSupplierName($supplier);
                 $supplierFile = ROOT . '/config/supplier_keys/' . $supplier . '.json';
                 if (!\file_exists($supplierFile)) {
                     throw new NoSupplier(