Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update notification #478

Merged
merged 3 commits into from
Jun 22, 2022
Merged

Update notification #478

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b026bb7
app: check for updates on start
rithvikvibhu Feb 28, 2022
00abe7c
i18n: extract string
rithvikvibhu Feb 28, 2022
b354c4e
app: harden update url
rithvikvibhu May 3, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions app/background/setting/client.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,5 @@ export const clientStub = ipcRendererInjector => makeClient(ipcRendererInjector,
'setLocale',
'getCustomLocale',
'setCustomLocale',
'getLatestRelease',
]);
16 changes: 16 additions & 0 deletions app/background/setting/service.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,21 @@ export async function setCustomLocale(json) {
return await put(CUSTOM_LOCALE, JSON.stringify(json));
}

export async function getLatestRelease() {
try {
const releases = await (
await fetch(
'https://api.github.com/repos/kyokan/bob-wallet/releases'
Copy link
Contributor

@pinheadmz pinheadmz Apr 20, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mine as well check some kind of signature, even if its a brand new bob-release-update published by kyokan. Especially because you use url: latestRelease.html_url, down below. Giving users a link to click is a target for attacks, treat it like a nuclear bomb.

https://bitcoinist.com/electrum-wallet-phishing-bitcoin/

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow didn't think of that. Unlikely, but def possible.

How about always linking to https://github.com/kyokan/bob-wallet/releases/tag/{TAG} instead of latestRelease.html_url?

Not sure what signature can be checked here. Do you mean like with PGP? It's just a link so no info to verify.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Look around how other popular wallets like Electrum and Wasabi handle this. Hard coding the link feels good. As far as signature what I meant was that Kyokan should publish a new key (hey maybe an HNS address so we can use rpc verify message) and Bob will verify an update notification from whatever source. I dunno if it's possible to check that the release signature on github is correct before notifying the user. Whatever works out, just keep in mind attackers would love to get Bob users to download malware with one button click, so line up the troops.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to prefix url and only use tag. imo the only way to break this would be to MITM a HTTPS req to github.com, which if someone can do, there are bigger problems already.

maybe an HNS address so we can use rpc verify message

Requires the node to be synced :|

I dunno if it's possible to check that the release signature on github is correct before notifying the user.

Afaik, no. We could download another file and check that, but again relies on an external source, mostly over https, with the same attack vectors as https://github.com. Additionally, verifying a file would mean downloading the actual binary from within Bob which feels more dangerous.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requires the node to be synced :|

no, just the address, not the NAME.

)
).json();
const latest = releases.filter(r => !r.draft && !r.prerelease)[0];
return latest;
} catch (error) {
console.error(error);
return null;
}
}

const sName = 'Setting';
const methods = {
getExplorer,
Expand All @@ -45,6 +60,7 @@ const methods = {
setLocale,
getCustomLocale,
setCustomLocale,
getLatestRelease,
};

export async function start(server) {
Expand Down
23 changes: 20 additions & 3 deletions app/components/Sidebar/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { shell } from 'electron';
import React, { Component } from 'react';
import { withRouter, NavLink } from 'react-router-dom';
import PropTypes from 'prop-types';
Expand All @@ -22,6 +23,7 @@ const nodeClient = clientStub(() => require('electron').ipcRenderer);
walletSync: state.wallet.walletSync,
walletHeight: state.wallet.walletHeight,
address: state.wallet.address,
updateAvailable: state.app.updateAvailable,
}),
dispatch => ({

Expand All @@ -44,6 +46,7 @@ class Sidebar extends Component {
walletHeight: PropTypes.number.isRequired,
network: PropTypes.string.isRequired,
address: PropTypes.string.isRequired,
updateAvailable: PropTypes.object,
};

static contextType = I18nContext;
Expand Down Expand Up @@ -199,13 +202,27 @@ class Sidebar extends Component {
newBlockStatus,
chainHeight,
tip,
updateAvailable,
} = this.props;

return (
<div className="sidebar__footer">
<div className="sidebar__footer__row">
<div className="sidebar__footer__title">{newBlockStatus}</div>
</div>
{updateAvailable ? (
<div className="sidebar__footer__row">
<button
className="sidebar__footer__update-notif"
onClick={() => shell.openExternal(updateAvailable.url)}
>
{t('updateAvailable')} ({updateAvailable.version})
</button>
</div>
) : null}
{newBlockStatus ? (
<div className="sidebar__footer__row">
<div className="sidebar__footer__title">{newBlockStatus}</div>
</div>)
: null
}
<div className="sidebar__footer__row">
<div className="sidebar__footer__title">{t('currentHeight')}</div>
<div className="sidebar__footer__text">
Expand Down
11 changes: 9 additions & 2 deletions app/components/Sidebar/sidebar.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
@import '../../variables';
@import '../../variables.scss';

.sidebar {
@extend %col-nowrap;
Expand Down Expand Up @@ -79,7 +79,7 @@

&__footer {
@extend %col-nowrap;
padding-bottom: 1rem;
padding: 0.3rem 0rem;
position: relative;
flex: 0 0 auto;
border-top: 1px solid rgba($black, 0.1);
Expand Down Expand Up @@ -115,6 +115,13 @@
color: #333;
}
}

&__update-notif {
@extend %h5;
@extend %btn-secondary;
padding: 0.5rem;
text-align: center;
}
}

&__search {
Expand Down
25 changes: 25 additions & 0 deletions app/ducks/app.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import semver from 'semver';
const pkg = require('../../package.json');
import settingsClient from "../utils/settingsClient";
import hip2Client from '../utils/hip2Client';
import { SET_HIP2_PORT } from "./hip2Reducer";
Expand All @@ -6,12 +8,30 @@ const SET_DEEPLINK = 'app/setDeeplink';
const SET_LOCALE = 'app/setLocale';
const SET_CUSTOM_LOCALE = 'app/setCustomLocale';
const SET_DEEPLINK_PARAMS = 'app/setDeeplinkParams';
const SET_UPDATE_AVAILABLE = 'app/setUpdateAvailable';

const initialState = {
deeplink: '',
deeplinkParams: {},
locale: '',
customLocale: null,
updateAvailable: null,
};

export const checkForUpdates = () => async (dispatch) => {
const latestRelease = await settingsClient.getLatestRelease();
if (!latestRelease) return;

const canUpdate = semver.gt(latestRelease.tag_name.replace(/$v/i, ''), pkg.version);
if (canUpdate) {
dispatch({
type: SET_UPDATE_AVAILABLE,
payload: {
version: latestRelease.tag_name,
url: `https://github.com/kyokan/bob-wallet/releases/tag/${latestRelease.tag_name}`,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, good change.

},
});
}
};

export const initHip2 = () => async (dispatch) => {
Expand Down Expand Up @@ -103,6 +123,11 @@ export default function appReducer(state = initialState, action) {
...state,
customLocale: action.payload,
};
case SET_UPDATE_AVAILABLE:
return {
...state,
updateAvailable: action.payload,
};
default:
return state;
}
Expand Down
4 changes: 3 additions & 1 deletion app/pages/App/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ import AppHeader from "../AppHeader";
import Exchange from '../Exchange';
import SignMessage from "../SignMessage";
import VerifyMessage from "../VerifyMessage";
import {fetchLocale, initHip2} from "../../ducks/app";
import {fetchLocale, initHip2, checkForUpdates} from "../../ducks/app";
import {I18nContext} from "../../utils/i18n";
const connClient = cClientStub(() => require('electron').ipcRenderer);
const settingClient = sClientStub(() => require('electron').ipcRenderer);
Expand All @@ -49,6 +49,7 @@ const settingClient = sClientStub(() => require('electron').ipcRenderer);
(dispatch) => ({
initHip2: () => dispatch(initHip2()),
setExplorer: (explorer) => dispatch(nodeActions.setExplorer(explorer)),
checkForUpdates: () => dispatch(checkForUpdates()),
fetchLocale: () => dispatch(fetchLocale()),
}),
)
Expand Down Expand Up @@ -77,6 +78,7 @@ class App extends Component {
async componentDidMount() {
this.setState({isLoading: true});
this.props.fetchLocale();
this.props.checkForUpdates();
await this.props.startNode();
await this.props.initHip2();
this.props.watchActivity();
Expand Down
1 change: 1 addition & 0 deletions locales/en.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -531,6 +531,7 @@
"unknownBid": "Unknown Bid",
"unlockWallet": "Unlock Wallet",
"update": "Update",
"updateAvailable": "Update Available",
"uploadAuctionFile": "Upload Auction File",
"userDirectory": "User Directory",
"verify": "Verify",
Expand Down