Skip to content
This repository has been archived by the owner on Oct 28, 2022. It is now read-only.

Commit

Permalink
Validate custom spend limit (MetaMask#7920)
Browse files Browse the repository at this point in the history
The custom spend limit was previously not validated. It did have a
minimum of zero set, but this didn't have any affect (that minimum is
used for form constraint validation, and this field wasn't in a form).
The field was never checked to ensure the contents didn't exceed the
maximum.

The field is now checked for values that exceed the maximum, and
invalid values in general (including negative values).

The parameters to the `showEditApprovalPermissionModal` were also
alphabetized to make them easier to read. In the course of doing this,
I noticed that the origin was missing from one of the calls. This was
responsible for the modal saying "Spend limit requested by undefined"
when clicking "Edit" under the transaction details. This has been
fixed.
  • Loading branch information
Gudahtt authored and yqrashawn committed Feb 3, 2020
1 parent e0a5765 commit 58e98b1
Show file tree
Hide file tree
Showing 5 changed files with 110 additions and 61 deletions.
6 changes: 6 additions & 0 deletions app/_locales/en/messages.json
Original file line number Diff line number Diff line change
Expand Up @@ -1313,6 +1313,12 @@
"message": "Spend limit requested by $1",
"description": "Origin of the site requesting the spend limit"
},
"spendLimitTooLarge": {
"message": "Spend limit too large"
},
"spendLimitInvalid": {
"message": "Spend limit invalid; must be a positive number"
},
"switchNetworks": {
"message": "Switch Networks"
},
Expand Down
Original file line number Diff line number Diff line change
@@ -1,21 +1,26 @@
import React, { PureComponent } from 'react'
import PropTypes from 'prop-types'
import log from 'loglevel'
import Modal from '../../modal'
import Identicon from '../../../ui/identicon'
import TextField from '../../../ui/text-field'
import { calcTokenAmount } from '../../../../helpers/utils/token-util'
import classnames from 'classnames'
import BigNumber from 'bignumber.js'

const MAX_UNSIGNED_256_INT = new BigNumber(2).pow(256).minus(1).toString(10)

export default class EditApprovalPermission extends PureComponent {
static propTypes = {
decimals: PropTypes.number,
hideModal: PropTypes.func.isRequired,
selectedIdentity: PropTypes.object,
tokenAmount: PropTypes.string,
customTokenAmount: PropTypes.string,
tokenSymbol: PropTypes.string,
tokenBalance: PropTypes.string,
setCustomAmount: PropTypes.func,
origin: PropTypes.string,
origin: PropTypes.string.isRequired,
}

static contextTypes = {
Expand All @@ -27,7 +32,7 @@ export default class EditApprovalPermission extends PureComponent {
selectedOptionIsUnlimited: !this.props.customTokenAmount,
}

renderModalContent () {
renderModalContent (error) {
const { t } = this.context
const {
hideModal,
Expand Down Expand Up @@ -141,7 +146,6 @@ export default class EditApprovalPermission extends PureComponent {
<div className="edit-approval-permission__edit-section__option-input">
<TextField
type="number"
min="0"
placeholder={ `${Number(customTokenAmount || tokenAmount)} ${tokenSymbol}` }
onChange={(event) => {
this.setState({ customSpendLimit: event.target.value })
Expand All @@ -151,7 +155,8 @@ export default class EditApprovalPermission extends PureComponent {
}}
fullWidth
margin="dense"
value={this.state.customSpendLimit}
value={ this.state.customSpendLimit }
error={error}
/>
</div>
</div>
Expand All @@ -161,10 +166,44 @@ export default class EditApprovalPermission extends PureComponent {
)
}

validateSpendLimit () {
const { t } = this.context
const { decimals } = this.props
const { selectedOptionIsUnlimited, customSpendLimit } = this.state

if (selectedOptionIsUnlimited || !customSpendLimit) {
return
}

let customSpendLimitNumber
try {
customSpendLimitNumber = new BigNumber(customSpendLimit)
} catch (error) {
log.debug(`Error converting '${customSpendLimit}' to BigNumber:`, error)
return t('spendLimitInvalid')
}

if (customSpendLimitNumber.isNegative()) {
return t('spendLimitInvalid')
}

const maxTokenAmount = calcTokenAmount(MAX_UNSIGNED_256_INT, decimals)
if (customSpendLimitNumber.greaterThan(maxTokenAmount)) {
return t('spendLimitTooLarge')
}
}

render () {
const { t } = this.context
const { setCustomAmount, hideModal, customTokenAmount } = this.props
const { selectedOptionIsUnlimited, customSpendLimit } = this.state

const error = this.validateSpendLimit()
const disabled = Boolean(
(customSpendLimit === customTokenAmount && !selectedOptionIsUnlimited) ||
error
)

return (
<Modal
onSubmit={() => {
Expand All @@ -175,11 +214,9 @@ export default class EditApprovalPermission extends PureComponent {
submitType="primary"
contentClass="edit-approval-permission-modal-content"
containerClass="edit-approval-permission-modal-container"
submitDisabled={
customSpendLimit === customTokenAmount && !selectedOptionIsUnlimited
}
submitDisabled={disabled}
>
{this.renderModalContent()}
{ this.renderModalContent(error) }
</Modal>
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ export default class ConfirmApproveContent extends Component {
}

static propTypes = {
decimals: PropTypes.number,
tokenAmount: PropTypes.string,
customTokenAmount: PropTypes.string,
tokenSymbol: PropTypes.string,
Expand Down Expand Up @@ -146,6 +147,7 @@ export default class ConfirmApproveContent extends Component {
render () {
const { t } = this.context
const {
decimals,
siteImage,
tokenAmount,
customTokenAmount,
Expand Down Expand Up @@ -181,16 +183,15 @@ export default class ConfirmApproveContent extends Component {
<div className="confirm-approve-content__edit-submission-button-container">
<div
className="confirm-approve-content__medium-link-text cursor-pointer"
onClick={() =>
showEditApprovalPermissionModal({
customTokenAmount,
tokenAmount,
tokenSymbol,
setCustomAmount,
tokenBalance,
origin,
})
}
onClick={() => showEditApprovalPermissionModal({
customTokenAmount,
decimals,
origin,
setCustomAmount,
tokenAmount,
tokenSymbol,
tokenBalance,
})}
>
{t('editPermission')}
</div>
Expand Down Expand Up @@ -228,34 +229,39 @@ export default class ConfirmApproveContent extends Component {
})}
</div>

{showFullTxDetails ? (
<div className="confirm-approve-content__full-tx-content">
<div className="confirm-approve-content__permission">
{this.renderApproveContentCard({
symbol: <img src="/images/user-check.svg" />,
title: 'Permission',
content: this.renderPermissionContent(),
showEdit: true,
onEditClick: () =>
showEditApprovalPermissionModal({
customTokenAmount,
tokenAmount,
tokenSymbol,
tokenBalance,
setCustomAmount,
}),
})}
</div>
<div className="confirm-approve-content__data">
{this.renderApproveContentCard({
symbol: <i className="fa fa-file" />,
title: 'Data',
content: this.renderDataContent(),
noBorder: true,
})}
</div>
</div>
) : null}
{
showFullTxDetails
? (
<div className="confirm-approve-content__full-tx-content">
<div className="confirm-approve-content__permission">
{this.renderApproveContentCard({
symbol: <img src="/images/user-check.svg" />,
title: 'Permission',
content: this.renderPermissionContent(),
showEdit: true,
onEditClick: () => showEditApprovalPermissionModal({
customTokenAmount,
decimals,
origin,
setCustomAmount,
tokenAmount,
tokenSymbol,
tokenBalance,
}),
})}
</div>
<div className="confirm-approve-content__data">
{this.renderApproveContentCard({
symbol: <i className="fa fa-file" />,
title: 'Data',
content: this.renderDataContent(),
noBorder: true,
})}
</div>
</div>
)
: null
}
</div>
)
}
Expand Down
1 change: 1 addition & 0 deletions ui/app/pages/confirm-approve/confirm-approve.component.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ export default class ConfirmApprove extends Component {
title={tokensText}
contentComponent={(
<ConfirmApproveContent
decimals={decimals}
siteImage={siteImage}
setCustomAmount={(newAmount) => {
this.setState({ customPermissionAmount: newAmount })
Expand Down
29 changes: 14 additions & 15 deletions ui/app/pages/confirm-approve/confirm-approve.container.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,24 +90,23 @@ const mapDispatchToProps = dispatch => {
showCustomizeGasModal: txData =>
dispatch(showModal({ name: 'CUSTOMIZE_GAS', txData })),
showEditApprovalPermissionModal: ({
tokenAmount,
customTokenAmount,
tokenSymbol,
tokenBalance,
decimals,
origin,
setCustomAmount,
tokenAmount,
tokenBalance,
tokenSymbol,
}) => dispatch(showModal({
name: 'EDIT_APPROVAL_PERMISSION',
customTokenAmount,
decimals,
origin,
}) =>
dispatch(
showModal({
name: 'EDIT_APPROVAL_PERMISSION',
tokenAmount,
customTokenAmount,
tokenSymbol,
tokenBalance,
setCustomAmount,
origin,
})
),
setCustomAmount,
tokenAmount,
tokenBalance,
tokenSymbol,
})),
}
}

Expand Down

0 comments on commit 58e98b1

Please sign in to comment.