7 ms·

> * Before they could start, we had a very serious security vulnerability responsibly disclosed by Will Farrington and Charlie Somerville > * We fixed it on Fe

by cyphunk 13y ago

> * Before they could start, we had a very serious security vulnerability responsibly disclosed by Will Farrington and Charlie Somerville > * We fixed it on February 17th the fix scares the shit out of me: https://github.com/isaacs/st/commit/5a0c1886737a20d78ae00b61e4724ae3095f4ddd https://github.com/isaacs/st/commit/5a0c1886737a20d78ae00b61... Properly escape all relevant html entities Avoid problems with files named things like '<img>' and so on. - var name = f.replace(/"/g, '"') + var name = f + .replace(/"/g, '"') + .replace(/>/g, '<') + .replace(/</g, '>') + .replace(/'/g, ''')

InclinedPlane 13y ago
Holy. Fucking. Crap. Jesus tapdancing christ that is seriously scary to see in something that's allegedly "all totally secure now, for really reals". More so the fact that such simple sanitization was missing for so long. Well, I guess I'll put off learning node a bit longer then.
matt_kantor 13y ago
It looks like this change has more to do with XSS than the "big" exploit. The more serious fix occurred here: https://github.com/isaacs/st/commit/6b54ce2d2fb912eadd31e2c25c65456d2c8666e1 https://github.com/isaacs/st/commit/6b54ce2d2fb912eadd31e2c2... And here: https://github.com/isaacs/st/commit/6d6100eec8b19e2774a6f2bb5c9b54fa9e1f9e72#diff-02be450bd9337a4b4e26c2139550120bR160 https://github.com/isaacs/st/commit/6d6100eec8b19e2774a6f2bb... With some icing on the cake here: https://github.com/isaacs/st/commit/8b2f212f64b762e351f311f4bbfcb291aa997838 https://github.com/isaacs/st/commit/8b2f212f64b762e351f311f4...
- cyphunk 13y ago
  not much more encouraging. it looks to me like patch work. ive had this in the past. would give a PoC to a client along with a recommended design change to the questionable methods of the code. they would send back a new version with a patch much like all of those linked here. in the end those patches address the PoC but not the problem. then i just rework the PoC to go around the patch. This cat-mouse game goes on until they go back, do the f'ing work, and implement the original design change suggested. I say all that just to point out that this looks like patch work and is a scary behaviour. Then again, maybe this is the nature of nodejs (omg). Also, as a general rule: ANY SECURITY PATCH THAT IS A REGEX IS NOT A SECURITY PATCH
  - hueving 13y ago
    >ANY SECURITY PATCH THAT IS A REGEX IS NOT A SECURITY PATCH Not true. Regex works well for forcing integers.

by cyphunk 13y ago

InclinedPlane 13y ago

Holy. Fucking. Crap. Jesus tapdancing christ that is seriously scary to see in something that's allegedly "all totally secure now, for really reals". More so the fact that such simple sanitization was missing for so long. Well, I guess I'll put off learning node a bit longer then.

matt_kantor 13y ago

It looks like this change has more to do with XSS than the "big" exploit. The more serious fix occurred here: https://github.com/isaacs/st/commit/6b54ce2d2fb912eadd31e2c25c65456d2c8666e1 https://github.com/isaacs/st/commit/6b54ce2d2fb912eadd31e2c2... And here: https://github.com/isaacs/st/commit/6d6100eec8b19e2774a6f2bb5c9b54fa9e1f9e72#diff-02be450bd9337a4b4e26c2139550120bR160 https://github.com/isaacs/st/commit/6d6100eec8b19e2774a6f2bb... With some icing on the cake here: https://github.com/isaacs/st/commit/8b2f212f64b762e351f311f4bbfcb291aa997838 https://github.com/isaacs/st/commit/8b2f212f64b762e351f311f4...

cyphunk 13y ago
not much more encouraging. it looks to me like patch work. ive had this in the past. would give a PoC to a client along with a recommended design change to the questionable methods of the code. they would send back a new version with a patch much like all of those linked here. in the end those patches address the PoC but not the problem. then i just rework the PoC to go around the patch. This cat-mouse game goes on until they go back, do the f'ing work, and implement the original design change suggested. I say all that just to point out that this looks like patch work and is a scary behaviour. Then again, maybe this is the nature of nodejs (omg). Also, as a general rule: ANY SECURITY PATCH THAT IS A REGEX IS NOT A SECURITY PATCH
- hueving 13y ago
  >ANY SECURITY PATCH THAT IS A REGEX IS NOT A SECURITY PATCH Not true. Regex works well for forcing integers.