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.

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

Fixed deprecated function for security reason #9

Merged
merged 1 commit into from
Nov 10, 2017
Merged

Fixed deprecated function for security reason #9

merged 1 commit into from
Nov 10, 2017

Conversation

darai0512
Copy link
Contributor

@darai0512 darai0512 commented Nov 9, 2017

@wozozo

概要

new Buffer()における初期化はdeprecatedとなり、Buffer.from()による初期化が推奨されています。
https://nodejs.org/dist/latest-v9.x/docs/api/buffer.html#buffer_new_buffer_array

理由としては、new Buffer()の引数には文字列だけでなく数値も許しており、
数値の場合、0 fillオプションを付けていないと初期化されていない領域を確保しようとします。

$node -v
v6.0.0
$cat deprecated.js
const sec = 'mysecretpassword';
while(1) {
    let string = (new Buffer(1024)).toString();
    if(/sec/.test(string)) {
        console.log(string);
        break;
    }
}
$node deprecated.js
... ��GmysecretpasswordH\贃8��&�prototypeH\�# ...

リスク

既存のnew Buffer()の引数にはTemplate literalで確実に文字列が渡されており、本修正を適用しなくても問題ありません。
利用者側の心理的安全と、今後の改修で誤らないよう(例えばapikeyが数値となったのでTemplate literalを外した、など)保険をかける程度のメリットとなります。

制限事項

v4.5.0以下にはバックポートされていないため、利用者はバージョンの制約を受けます。
.travis.ymlでは4系以上のみ保証しているので、心配無用?)

test/lint/build

確認済。
testにはnew Buffer()を残していますが、今回の変更が既存の処理に影響ないことを示すためです。

Changed to Buffer.from() because new Buffer() is deprecated.
@wozozo wozozo merged commit c51d465 into payjp:master Nov 10, 2017
@wozozo
Copy link
Contributor

wozozo commented Nov 10, 2017

🙇‍♀️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants