Saving Airship settings with "Notarize Updates for other Airships?" causes Hull to fail to load, no validation of user input?

[alexdenvir]

Summary

Using current master branch

I go to https://url.tld/bridge/admin/settings, check "Notarize Updates for other Airships?", and click "Save Settings".
Next, I log out of Bridge, and I restart php fpm on my server.
I then go to https://url.tld to find nginx gives me a 500 error.

Further investigation suggests that when I saved the settings, config/universal.json was updated and saved null for notary/channel. Digging through the nginx error logs I find a fatal error, caused by \Airship\Engine\Security\Util::noHTML having its first parameter typed to string, but receiving null instead.

Manually updating universal.json to set the channel back to paragonie, and restarting php fpm fixes this.

Expected Outcome

universal.json is updated correctly, and navigating to Hull should not trigger a fatal error

What Actually Happened

universal.json is updated, but notary config is "incomplete"

{
    /* Universal Configuration for an Airship deployment */
    "airship": {
        "trusted-supplier": "paragonie"
    },
    "auto-update": {
        "ignore-peer-verification": false,
        "check": 3600,
        "major": false,
        "minor": true,
        "patch": true,
        "test":  false
    },
    "cookie_index": {
        "auth_token": "airship_token"
    },
    "debug": false,
    "email": {
        "from": null
    },
    "guest_groups": [
        1
    ],
    "ledger": {
        "driver": "file",
        "path":   "\/tmp\/airship.log"
    },
    "guzzle": [],
    "notary": {
        "channel": null,
        "enabled": true
    },
    "session_config": {
    "cookie_domain": ""
},
    "session_index": {
        "user_id": "userid",
        "logout_token": "logout_token"
    },
    "tor-only": false,
    "twig-cache":  true
}

Stack trace from nginx error log

PHP message: PHP Fatal error:  Uncaught TypeError: Argument 1 passed to Airship\Engine\Security\Util::noHTML() must be of the type string, null given, called in /{path}/src/lens_functions.php on line 317 and defined in /{path}/src/Engine/Security/Util.php:34
Stack trace:
#0 /{path}/src/lens_functions.php(317): Airship\Engine\Security\Util::noHTML(NULL)
#1 /{path}/vendor/twig/twig/lib/Twig/Environment.php(403) : eval()'d code(65): Airship\LensFunctions\display_notary_tag()
#2 /{path}/vendor/twig/twig/lib/Twig/Template.php(167): __TwigTemplate_35bb9ec878691b75b1629a21ef6449901f542f500bcc9d2f8ce3c2c7ffb6927a->block_head(Array, Array)
#3 /{path}/vendor/twig/twig/lib/Twig/Environment.php(403) : eval()'d code(32): Twig_Template->displayBlock('head', Array, Array)
#4 /{path}/vendor/twig/twig/lib/Twig/Template.php(387): __TwigTemplate_35bb9ec878691b75b1629a21ef6449901f542f500bcc9d2f8ce3c2c7ffb6927a->doDisplay(Array, Array)

Common Issues

Please make sure all these boxes are checked before you submit your issue.

• You are running a supported version of PHP (7.0.x) (Run php -v from the command line)
• You have libsodium 1.0.9 or newer installed
• You have version 1.0.3 or newer of the PHP extension for libsodium installed

[paragonie-scott]

Are you using master or the 0.2.1 release?

[alexdenvir]

master

[paragonie-scott]

Okay, thanks. I'll be sure to fix this ASAP.

[alexdenvir]

I started to look into making my own patch this, and it looks like the post data from that form is being used without any validation (other than anti csrf stuff) to build the json file.

Is there anything like an input filter (like what Zend Framework and other large frameworks use), or perhaps a plan to implent something along those lines in the future? (my thinking is that by validating the post input, you could catch the missing key and fail early, before writing an incomplete config)

[paragonie-scott]

Yes, that's something we're going to be doing for v0.3.0. Among other concerns: Some fields are null and HTTP passes "1" or empty instead of true/false for checkboxes.

[paragonie-scott]

The fix for this might not land for a few days, currently working on a rather large piece. This will be the immediate next thing to address.

Thanks a ton for reporting this. 👍

[alexdenvir]

No problem :)
It's always fun contributing to new projects

On Tue, 14 Jun 2016, 17:32 Scott, notifications@github.com wrote:

The fix for this might not land for a few days, currently working on a
rather large piece. This will be the immediate next thing to address.

Thanks a ton for reporting this. 👍

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#33 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/ABWL6tVoIMGPykSc8Vl54c2WMQvQd5LDks5qLte1gaJpZM4I1f9u
.

[paragonie-scott]

a18ff04 + cc94639

[alexdenvir]

👍 looking good

[paragonie-scott]

cfdf85c ought to fix it totally (and remove the early stages of a less-well-thought-out attempt to solve the same problem).

Thanks a ton for reporting this. 😄