Closed laino closed 6 years ago
Is there an issue here? I didn't use chalk/templates
as it's not part of the supported API.
Yes.
To be honest there is a million ways this can go wrong besides not being friendly to optimization by the VM and being hacky.
Here's a fun exploit:
const chalk = require('chalk')
const evilString = "\\` + (console.log('Hi There!'));//";
chalkFormat(evilString);
function chalkFormat (str) {
str = str.replace(/`/g, '\\`');
const format = new Function('chalk', `'use strict'; return chalk\`${str}\``);
return format(chalk);
}
why would a developer using command-line-usage feed it an evil string?
It doesn't have to be a string being intentionally evil (though it might be if you're populating your strings with stuff from your web fronted, for instance for user management).
Much more likely people will unintentionally break it by supplying some bad value.
Right now it's just super annoying because doing it this way removes any double backslashes twice, so
I have to do "\\\\{not a template\\\\\}"
- which also doesn't work because in some places the string gets run through chalkFormat twice in a row, making escaping templates impossible.
Edit: That above example also doesn't work, you have to use six \
, but then one gets reproduced in the final output.
Edit2:
const chalk = require('chalk');
const string = "\\\\\\{send help\\\\\\}";
console.log(chalkFormat(string));
function chalkFormat (str) {
str = str.replace(/`/g, '\\`');
const format = new Function('chalk', `'use strict'; return chalk\`${str}\``);
return format(chalk);
}
Edit3: It actually somehow works with only two \
when using eval/Function, for reasons I can't fathom.
i don't think chalk yet supports escaping curly braces but still, the benefits outweighed the lack of this feature (which could still be added).
On 13 Mar 2018 06:19, "Nils Kuhnhenn" notifications@github.com wrote:
It doesn't have to be a string being intentionally evil (though it might be if you're populating your strings with stuff from your web fronted, for instance for user management).
Much more likely people will unintentionally break it by supplying some bad value.
Right now it's just super annoying because doing it this way removes any double backslashes twice, so I have to do "\\{not a template\\}" - which also doesn't work because in some places the string gets run through chalkFormat twice in a row, making escaping templates impossible.
— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/75lb/command-line-usage/issues/15#issuecomment-372504960, or mute the thread https://github.com/notifications/unsubscribe-auth/ABMK7BqAH-nmZ3PBRDd9oFtBGmutBv0mks5tdxCogaJpZM4Sn3V6 .
Escaping curly braces appears to work:
const chalk = require('chalk');
const chalkTemplate = require("chalk/templates");
const myString = "\\{red RED\\}";
console.log("Works:", chalkTemplate(chalk, myString));
Turns out it also works with the eval
variant, as long as I'm using it directly.
Right now it just doesn't work with command-line-usage
because all options get run through
chalkFormat
twice, causing errors on the second run.
Fine. How about this then:
const chalk = require('chalk');
const myString = "{red RED}";
console.log("Works:", chalkFormat(myString));
function chalkFormat(string) {
const arr = [string];
arr.raw = [string];
return chalk(arr);
}
That uses the ES2015 template literal API (not 100% to spec, but it works) and it doesn't use any undocumented chalk features.
If you'd want to do it to spec you'd have to go and replace all all escape sequences in raw
with their
literal values, but chalk handles that fine anyways.
Ok, will have a look into your escaping suggestion ASAP.. i'm on holiday atm but will get to this within 24 hours..
escaping fixed and released in v5.0.3. You were right, chalkFormat
was being applied twice.
i'm back from holiday now (although still jet-lagged!) and have implemented the raw
property solution in v5.0.4, as suggested.. thanks again, keep me posted.
Reference
Consider running stuff through the chalk template function like this:
Which is slightly less ugly, but still hacky.
Also see https://github.com/chalk/chalk/issues/258