tronprotocol / tips

TRON Improvement Proposals
229 stars 203 forks source link

TIP-534: Remove Vulnerable APIs #534

Closed halibobo1205 closed 1 year ago

halibobo1205 commented 1 year ago
tip: 534
title: Remove Vulnerable APIs   
author: halibobo1205@gmail.com
discussions to: https://github.com/tronprotocol/TIPs/issues/534
status: Final
type: Standards Track
category: Core
created: 2023-04-014

Simple Summary

Propose to remove potential vulnerable APIs.

Abstract

Previously some APIs could lead to the leakage of sensitive information, such as private keys. Although developers have been advised not to make remote calls to these APIs, I think it is better to remove the relevant APIs for security concerns and mitigate risks for developers.

Motivation

For network stability and user data security, I suggest deleting the relevant APIs.

Specification

The following APIs should be removed.

1. HTTP

API Description
createaddress create address by password
generateaddress create address randomly
easytransfer transfer TRX with password
easytransferbyprivate transfer TRX with private key
easytransferasset transfer asset with password
easytransferassetbyprivate transfer asset with private key
gettransactionsign sign transaction with private key
addtransactionsign sign transaction with private key

2. gGRPC

API Description
CreateAddress create address by password
GenerateAddress create address randomly
EasyTransfer transfer TRX with password
EasyTransferByPrivate transfer TRX with private key
EasyTransferAsset transfer asset with password
EasyTransferAssetByPrivate transfer asset with private key
GetTransactionSign sign transaction with private key
GetTransactionSign2 sign transaction with private key
AddSign sign transaction with private key

Rationale

TODO

Implementation

TODO

bractbright commented 1 year ago

No problem bro

txoh1603 commented 1 year ago

wow, this is as early as necessary

jernganl commented 1 year ago

I couldn't agree more

Jamestepfoward commented 1 year ago

Agree, but one question, if this proposal get approved by majority, how will this going to be enabled, by a new chain parameter? Or by a mandatory node upgrade?

I think mandatory upgrade is just fine, cause it is security issues only.

txoh1603 commented 1 year ago

@Jamestepfoward might be a small patch I guess

m-o-m12 commented 1 year ago

Although most people will do not use these api, but for some blockchain beginners, these api will be more convenience. So as long as the interface exists, someone will use it, and exposing the private key is not conducive to account security, it's better to remove them. Creating address locally, signing transaction locally should be promoted, but not by others.

souppopnix commented 1 year ago

Might be a silly question. Like m-o-m12 said, we use gettransactionsign locally all the time, by remove, do you mean remove this API completely or just for the public api? If the removal is permanent, what's the replacement if we want to use HTTP request rather than modules and sdks, say wallet-cli, tronweb module etc..

LauraYH commented 1 year ago

@souppopnix I think using HTTP requests to sign with PK is always a bad idea, PK stealing and revealing are furious recently, third party services stopped the API a long time ago. It seems for now we have to go with the SDK and modules.

405560984 commented 1 year ago

I couldn't agree more

ethan1844 commented 1 year ago

Close this issue as it is implemented by GreatVoyage-v4.7.1.1(Pittacus)