Register a SA Forums Account here!
JOINING THE SA FORUMS WILL REMOVE THIS BIG AD, THE ANNOYING UNDERLINED ADS, AND STUPID INTERSTITIAL ADS!!!

You can: log in, read the tech support FAQ, or request your lost password. This dumb message (and those ads) will appear on every screen until you register! Get rid of this crap by registering your own SA Forums Account and joining roughly 150,000 Goons, for the one-time price of $9.95! We charge money because it costs us money per month for bills, and since we don't believe in showing ads to our users, we try to make the money back through forum registrations.
 
  • Post
  • Reply
The Fool
Oct 16, 2003


I made a terrible thing last night.
code:
var express = require('express');
var exec = require('child_process').exec;

var app = express();

var port = 3000;

app.get('/:command/*?', function (req, res) {
  var command = "powershell.exe " + req.params.command;
  var args = req.params[0].replace(/\//i, ' ');
  command += " " + args;

  exec(command, function(error, stdout, stderr) {
    var result = "<h4>" + command + "</h4>";
    if (error) {
      result += error;
    } else {
      result += stdout;
    }
    res.send("<pre>" + result + "</pre>");
  });
});

app.listen(port, function () {
  console.log('Listening on port: ' + port);
});
For the sake of argument, what are some things that can be done to make this bit of code not a giant black hole of security risks?

Obvious ones to me:
Don't allow arbitrary execution of commands, only specific ones.
Make sure the node process is running as a service account with only the desired permissions.
Require authentication.

What else is there?

The Fool fucked around with this message at 19:27 on Dec 15, 2016

Adbot
ADBOT LOVES YOU

Sedro
Dec 31, 2008
You've got XSS and CSRF vulnerabilities too but who cares lol

Roadie
Jun 30, 2013

The Fool posted:

For the sake of argument, what are some things that can be done to make this bit of code not a giant black hole of security risks?

Nothing. Anything that lets people run arbitrary command-line code will gently caress you.

What do you actually want to do with this?

necrotic
Aug 2, 2005
I owe my brother big time for this!

The Fool posted:

Don't allow arbitrary execution of commands, only specific ones.
Make sure the node process is running as a service account with only the desired permissions.
Require authentication.

All of these. And the first shouldn't be a whitelist but a mapping, so like if you need to call something say shoot with a parameter you make a route /shoot/:param and then further validate that param (like maybe it only allows self or foot.

But yeah, why?

Maluco Marinero
Jan 18, 2001

Damn that's a
fine elephant.

Roadie posted:

Nothing. Anything that lets people run arbitrary command-line code will gently caress you.

What do you actually want to do with this?

For real, the only thing you could do for security is IP restrict access to the server, and severely lock down the access rights of the server that's running the shell code, and even then, that's a one plank across a gigantic and fundamental security issue.

The Fool
Oct 16, 2003


Roadie posted:

Nothing. Anything that lets people run arbitrary command-line code will gently caress you.

I'm aware of that, considering it was literally the first item in my list.

quote:

What do you actually want to do with this?

It's just a toy that isn't going to be run anywhere except my home lab. I do plan on putting a react app on top of this to select from a list of scripts in a folder, execute them, and display the results.

Roadie
Jun 30, 2013

The Fool posted:

It's just a toy that isn't going to be run anywhere except my home lab. I do plan on putting a react app on top of this to select from a list of scripts in a folder, execute them, and display the results.

In this case, code the scripts into the server, hard match against them, and never accept a parameter that you can't pass with strict binding as a self-contained string (think: directly calling function via C-to-node interfaces).

code:
function scriptname1 (params) {
  let cats = params.cats

  if (typeof cats === 'string') {
    callExternalLibraryHere(cats)
  } else {
    doOtherThingHere()
  }

  // TODO: do the thing
  return 'Output goes here'
}

function scriptname2 () {
  // TODO: do the thing
  return 'Output goes here'
}

function scriptname3 () {
  // TODO: do the thing
  return 'Output goes here'
}

app.get('/scripts/:script', function (req, res) {
  let output

  if (req.params.script === 'scriptname1') {
    output = scriptname1({thing1: req.query.cats})
  }

  if (req.params.script === 'scriptname2') {
    output = scriptname2()
  }

  if (req.params.script === 'scriptname3') {
    output = scriptname3()
  }

  if (output) {
    res.send(output)
  }
})

geeves
Sep 16, 2004

The Fool posted:

I made a terrible thing last night.
code:
var express = require('express');
var exec = require('child_process').exec;

var app = express();

var port = 3000;

app.get('/:command/*?', function (req, res) {
  var command = "powershell.exe " + req.params.command;
  var args = req.params[0].replace(/\//i, ' ');
  command += " " + args;

  exec(command, function(error, stdout, stderr) {
    var result = "<h4>" + command + "</h4>";
    if (error) {
      result += error;
    } else {
      result += stdout;
    }
    res.send("<pre>" + result + "</pre>");
  });
});

app.listen(port, function () {
  console.log('Listening on port: ' + port);
});
For the sake of argument, what are some things that can be done to make this bit of code not a giant black hole of security risks?

Obvious ones to me:
Don't allow arbitrary execution of commands, only specific ones.
Make sure the node process is running as a service account with only the desired permissions.
Require authentication.

What else is there?

Roadie posted:

Nothing. Anything that lets people run arbitrary command-line code will gently caress you.

What do you actually want to do with this?

Exactly.

#1 in java script: The Bad Parts or "DON'T loving DO THIS" - exec() (I realize this may be different in node, than in the browser - but until I know better/....)

If you are running a .exe (or .sh) make sure it's something airtight that you have also written yourself and you know exactly what you will be executing.

If you have to write sketchy code like this perhaps you should be handing off to another service / queuing system that can handle it properly.

Not to enable you (I don't know what powershell.exe does) at the very least, you should be checking if req.params.command are in fact valid and what you want to allow (for example, if it does CRUD and you only want Read, make sure the params are Read params - otherwise 403 / 500 that response). Treat everything like you would SQL injection and don't pass them blindly.

But as Roadie said, "No".

Roadie
Jun 30, 2013

geeves posted:

#1 in java script: The Bad Parts or "DON'T loving DO THIS" - exec() (I realize this may be different in node, than in the browser - but until I know better/....)

It's worse. In the browser, it at least (probably) can't delete all your files and email illegal porn to all your relatives if you gently caress it up.

The Fool
Oct 16, 2003


geeves posted:

(I don't know what powershell.exe does)

Imagine it is /usr/bin/python and you get the idea.

Knifegrab
Jul 30, 2014

Gadzooks! I'm terrified of this little child who is going to stab me with a knife. I must wrest the knife away from his control and therefore gain the upperhand.
I am loving garbo at bitwise operations. I have an integer, 8 bits, unsigned. I need to convert the base 10 integer into binary and check to see if the very first bit is set. But everything I try I fail at, anyone good at these sorts of things?

HappyHippo
Nov 19, 2003
Do you have an Air Miles Card?

Knifegrab posted:

I am loving garbo at bitwise operations. I have an integer, 8 bits, unsigned. I need to convert the base 10 integer into binary and check to see if the very first bit is set. But everything I try I fail at, anyone good at these sorts of things?

If I'm understanding you right, just do (num & 0x80) != 0? Or really just num >= 128.

HappyHippo fucked around with this message at 20:46 on Dec 20, 2016

Munkeymon
Aug 14, 2003

Motherfucker's got an
armor-piercing crowbar! Rigoddamndicu𝜆ous.



Knifegrab posted:

I am loving garbo at bitwise operations. I have an integer, 8 bits, unsigned. I need to convert the base 10 integer into binary and check to see if the very first bit is set. But everything I try I fail at, anyone good at these sorts of things?

!!(number & 1) should work if I understand what you're asking for correctly. !!(number & (1 << bit)) would be the general case (bit is zero-indexed).

Bitwise operations like &, |, << etc. all "convert" whatever you feed them to unsigned 32-bit integers under the hood by chopping off any decimals on a floating point number and IIRC any bits higher than 32, so they're not safe to use on Numbers larger than 232.

<< is a bit shift. It just moves the bits in an integer over and back-fills with zeroes.

!! just coerces the result to a boolean true or false.

peepsalot
Apr 24, 2007

        PEEP THIS...
           BITCH!

Knifegrab posted:

I am loving garbo at bitwise operations. I have an integer, 8 bits, unsigned. I need to convert the base 10 integer into binary and check to see if the very first bit is set. But everything I try I fail at, anyone good at these sorts of things?
What do you mean a base 10 integer? Base only come into play when talking about representing a number, not so much for the value itself, or doing operations on it. Are you dealing with a string of characters that represent a base 10 value?

Otherwise it's inconsequential how you assign a variable, the value is the same independent of base.

I mean, you can assign a value in base 10, 16 (0x prefix), 8 (0 prefix), or 2 (0b prefix):
var x = 10;
var y = 0xA;
var z = 012;
var w = 0b1010;

They are all equivalent.

Knifegrab
Jul 30, 2014

Gadzooks! I'm terrified of this little child who is going to stab me with a knife. I must wrest the knife away from his control and therefore gain the upperhand.

peepsalot posted:

What do you mean a base 10 integer? Base only come into play when talking about representing a number, not so much for the value itself, or doing operations on it. Are you dealing with a string of characters that represent a base 10 value?

Otherwise it's inconsequential how you assign a variable, the value is the same independent of base.

I mean, you can assign a value in base 10, 16 (0x prefix), 8 (0 prefix), or 2 (0b prefix):
var x = 10;
var y = 0xA;
var z = 012;
var w = 0b1010;

They are all equivalent.

Sorry I just mean that it is a normal integer, represented in standard base 10. Thanks for the help everyone, definitely solved the issue.

Knifegrab
Jul 30, 2014

Gadzooks! I'm terrified of this little child who is going to stab me with a knife. I must wrest the knife away from his control and therefore gain the upperhand.
So this is weird and frustrating, I've been working with node forever but all of a sudden if I console.log a large array, it will print quite a bit of it but then truncate the rest of it saying somethiung like:

code:
... 12 more items
This is bizarre. I'm working with big data and I need a lot of data output for debug, I've never seen this behavior before...

Even piping it to more doesn't solve the issue, I could probably pipe it to a file but that adds so much more hassle in terms of quick debug.

ddiddles
Oct 21, 2008

Roses are red, violets are blue, I'm a schizophrenic and so am I

Knifegrab posted:

So this is weird and frustrating, I've been working with node forever but all of a sudden if I console.log a large array, it will print quite a bit of it but then truncate the rest of it saying somethiung like:

code:
... 12 more items
This is bizarre. I'm working with big data and I need a lot of data output for debug, I've never seen this behavior before...

Even piping it to more doesn't solve the issue, I could probably pipe it to a file but that adds so much more hassle in terms of quick debug.

This might help?

https://nodejs.org/api/util.html

Specifically the util.inspect.defaultOptions section halfway down the page.

That was just some quick googling though, not familiar with node.

Strong Sauce
Jul 2, 2003

You know I am not really your father.





Knifegrab posted:

So this is weird and frustrating, I've been working with node forever but all of a sudden if I console.log a large array, it will print quite a bit of it but then truncate the rest of it saying somethiung like:

code:
... 12 more items
This is bizarre. I'm working with big data and I need a lot of data output for debug, I've never seen this behavior before...

Even piping it to more doesn't solve the issue, I could probably pipe it to a file but that adds so much more hassle in terms of quick debug.

this was introduced to remove a possible ddos vector
https://github.com/nodejs/node/issues/4905

fix is to use maxArrayLength parameter in util.inspect and console.log that out.

Munkeymon
Aug 14, 2003

Motherfucker's got an
armor-piercing crowbar! Rigoddamndicu𝜆ous.



Can anyone recommend a currency-safe arbitrary precision library?

necrotic
Aug 2, 2005
I owe my brother big time for this!
https://www.npmjs.com/package/money-math

But don't do money related math in JS unless it's like some dumb estimate.

geeves
Sep 16, 2004

necrotic posted:

https://www.npmjs.com/package/money-math

But don't do money related math in JS unless it's like some dumb estimate.

yeah. Just don't. when I had to do money-like numbers, I just sent it to the server instead. Much safer and not much longer.

Bruegels Fuckbooks
Sep 14, 2004

Now, listen - I know the two of you are very different from each other in a lot of ways, but you have to understand that as far as Grandpa's concerned, you're both pieces of shit! Yeah. I can prove it mathematically.

geeves posted:

yeah. Just don't. when I had to do money-like numbers, I just sent it to the server instead. Much safer and not much longer.

I was thinking about replying like this but then I remembered that node.js is a thing, so it's conceivable that node.js could be used by like banking sites or some poo poo.

Munkeymon
Aug 14, 2003

Motherfucker's got an
armor-piercing crowbar! Rigoddamndicu𝜆ous.



necrotic posted:

https://www.npmjs.com/package/money-math

But don't do money related math in JS unless it's like some dumb estimate.

It's only one calculation because some jurisdictions don't want a fee broken out and it's just for display so it should be OK

The back-end guys basically said "oh we'd just add the numbers together as floats :shrug:" soooo I'm doing the best I can here

Professor of Cats
Mar 22, 2009

I think what people are trying to say is it isn't that JS is bad at math, it is that doing any sort of math poo poo on the front end is dumb, unsafe and untrustworthy.

Estimates, quotes, etc are fine. But the actual data that will result in a transaction should come from the server because any data exposed to the front end can be manipulated.

Bruegels Fuckbooks
Sep 14, 2004

Now, listen - I know the two of you are very different from each other in a lot of ways, but you have to understand that as far as Grandpa's concerned, you're both pieces of shit! Yeah. I can prove it mathematically.

Professor of Cats posted:

I think what people are trying to say is it isn't that JS is bad at math, it is that doing any sort of math poo poo on the front end is dumb, unsafe and untrustworthy.

Estimates, quotes, etc are fine. But the actual data that will result in a transaction should come from the server because any data exposed to the front end can be manipulated.

In JS, there is no integer type - all numbers are IEEE 754 doubles, and it is impossible to represent .1 accurately using such a number- for instance, .1 + .2 !== .3 in javascript. This can cause issues with interest rate calculations and the display of numbers, especially over time (e.g. all the problems http://stackoverflow.com/questions/3730019/why-not-use-double-or-float-to-represent-currency are valid.)

If you're just using dollars, you can avoid this by storing in cents (e.g. instead of $5.24, use 524.) The two problems with this approach are
a) You need to decide whether you care about fractions of a cent
b) This doesn't work for all kinds of currency. Bitcoin in particular has lots of weird fractions.

The separate problem is that doing the math on the front end is a bad idea because it's client side, but I was sarcastically pointing out that
a) node.js exists so you can't necessarily make the assumption that JS is client side
b) there are actually problems with doing this regardless of whether the JS is running client/server side for the above reason
c) man, since node.js is so popular, i bet people really have this problem

Professor of Cats
Mar 22, 2009


Oh great point on the node.js part. C) - I'll bet you are correct.
And I guess I kind of glazed over the "precision library" part too, which probably indicates things like long term interest rate calculations.

Jabor
Jul 16, 2010

#1 Loser at SpaceChem

Bruegels Fuckbooks posted:

b) This doesn't work for all kinds of currency. Bitcoin in particular has lots of weird fractions.

for all 2.7 people in the world who care about using bitcoins, javascript is just fine because bitcoin is built on doubles anyway

Bruegels Fuckbooks
Sep 14, 2004

Now, listen - I know the two of you are very different from each other in a lot of ways, but you have to understand that as far as Grandpa's concerned, you're both pieces of shit! Yeah. I can prove it mathematically.

Jabor posted:

for all 2.7 people in the world who care about using bitcoins, javascript is just fine because bitcoin is built on doubles anyway

I googled around to try to figure out why that came to mind and I found this: https://en.bitcoin.it/wiki/Proper_Money_Handling_%28JSON-RPC%29.

Munkeymon
Aug 14, 2003

Motherfucker's got an
armor-piercing crowbar! Rigoddamndicu𝜆ous.



Professor of Cats posted:

I think what people are trying to say is it isn't that JS is bad at math, it is that doing any sort of math poo poo on the front end is dumb, unsafe and untrustworthy.

Estimates, quotes, etc are fine. But the actual data that will result in a transaction should come from the server because any data exposed to the front end can be manipulated.

I know all that and I'm trying to mitigate the potential problems caused by others' poor decisions. Just wanna add a couple of numbers without the UI obviously making GBS threads itself obviously, but this is JS so that's A Hard Problem :sigh:


lmao no this place is a real business that only deals with real money

ddiddles
Oct 21, 2008

Roses are red, violets are blue, I'm a schizophrenic and so am I
If anyone is interested, Anthony Alicea's nodejs course on Udemy is discounted to $10.

https://www.udemy.com/understand-nodejs/learn/v4/overview

I did his JS course, I really like how he takes the time to explain how things are working behind the scenes, rather than just typing some code and having you copy it. It really helped me understand the fundamentals of JS.

I'm doing his node course now, and it seems to be the same sort of philosophy, really enjoying it.

geeves
Sep 16, 2004

Bruegels Fuckbooks posted:

I was thinking about replying like this but then I remembered that node.js is a thing, so it's conceivable that node.js could be used by like banking sites or some poo poo.

The next solo Superman movie will be a remake of 3 and have Kevin hart as the villain working with Lex. He won't be collecting simple half pennies from banks. Who knows what he'll exploit with node.js.

I highly doubt any financial institution is using node.js for any calculation.

Maluco Marinero
Jan 18, 2001

Damn that's a
fine elephant.
However PayPal's new checkout/payment system is driven by Node on the server as far as I'm aware. I would assume it's not actually responsible for the transactions directly though.

Professor of Cats
Mar 22, 2009

Now that I think about it - math, money and JS is fine.

As stated, you convert to a normal integer/cent. (yes yes I know, javascript uses Number ie IEEE 754 float). But it has a precision of 2^50, it can handle most ranges you throw at it. And if you do it yourself, format the immediate results of each individual calculation to avoid rounding errors or ... just use the many math libraries out there that take care of avoiding these rounding and precision issues.

So yea, we are stuck with floating point/double for our numbers in JS but as I said above, if you calculate outside of floating point and round/format each calculation correctly, I'd wager in most cases you are fine.

Furthermore, floating point issue isn't limited to javascript - it is found in any language due to the architecture you are using (x86) and due to the standard itself; but over the past whatever years, many smart people have developed great techniques for handling this issue so we don't have to think about it.

For an extremely DRY rear end read; http://docs.oracle.com/cd/E19957-01/806-3568/ncg_goldberg.html
A great stack overflow question/answer: http://stackoverflow.com/questions/3730019/why-not-use-double-or-float-to-represent-currency

If someone thinks I'm way off base, please educate me but as of right now, I see zero issues with handling money scenarios that are out of the extraordinary.

e: Sorry, this came across as super snotty!!! It was more of a "what a minute" moments as I remembered I've dealt with this on other firmware in my past life. I could be wrong as I never dealt with money situations but other measurement precision stuff - I figure it would be in the same category though, right?

Professor of Cats fucked around with this message at 02:56 on Jan 5, 2017

necrotic
Aug 2, 2005
I owe my brother big time for this!
The major point is that JS lacks integers period, even with integer values working okay. Do money math with pure ints or BigDecimal Tia. Don't divide money in JS thanks.

Edit: which is pretty much what that so post says.

necrotic fucked around with this message at 05:28 on Jan 5, 2017

Knifegrab
Jul 30, 2014

Gadzooks! I'm terrified of this little child who is going to stab me with a knife. I must wrest the knife away from his control and therefore gain the upperhand.
I am running a node server and when a user hits a route a really intensive proccess occurs. Part of it involves querying a database but then there are a LOT of post-query actions that take place done in javascript.

The problem is, this process can take up to 20 seconds to complete (its very intensive) and during this time it is blocking all other requests made by any other uses (since node is single threaded I am assuming).

Is there some kind of syntax or something I can throw into one of my many for loops or something which basically says "Come back here after you have checked for other tasks"?

Skandranon
Sep 6, 2008
fucking stupid, dont listen to me

Knifegrab posted:

I am running a node server and when a user hits a route a really intensive proccess occurs. Part of it involves querying a database but then there are a LOT of post-query actions that take place done in javascript.

The problem is, this process can take up to 20 seconds to complete (its very intensive) and during this time it is blocking all other requests made by any other uses (since node is single threaded I am assuming).

Is there some kind of syntax or something I can throw into one of my many for loops or something which basically says "Come back here after you have checked for other tasks"?

Not in the simplest sense. JS Engines do not do anything else until it's current function call exits (see https://developer.mozilla.org/en-US/docs/Web/JavaScript/EventLoop). What you want to do is either use a thread library to specifically create child processes to handle your task, or design your processing code such that it can be worked on in parts. Once you can queue up parts, use setTimeout to queue the next part. This will then allow the EventLoop to complete, and handle other tasks that have been building up in the queue.

Knifegrab
Jul 30, 2014

Gadzooks! I'm terrified of this little child who is going to stab me with a knife. I must wrest the knife away from his control and therefore gain the upperhand.

Skandranon posted:

Not in the simplest sense. JS Engines do not do anything else until it's current function call exits (see https://developer.mozilla.org/en-US/docs/Web/JavaScript/EventLoop). What you want to do is either use a thread library to specifically create child processes to handle your task, or design your processing code such that it can be worked on in parts. Once you can queue up parts, use setTimeout to queue the next part. This will then allow the EventLoop to complete, and handle other tasks that have been building up in the queue.

How would I do it promise based? I am using node and native Promises, what is best practice? I have tried just wrapping the data in a Promise.resolve() and reutrning it like a normal promise, but it doesn't seem to break out of the process, and it still has to complete fully before returning pages to other users.

Knifegrab fucked around with this message at 23:49 on Jan 12, 2017

Skandranon
Sep 6, 2008
fucking stupid, dont listen to me

Knifegrab posted:

How would I do it promise based? I am using node and native Promises, what is best practice? I have tried just wrapping the data in a Promise.resolve() and reutrning it like a normal promise, but it doesn't seem to break out of the process, and it still has to complete fully before returning pages to other users.

A Promise doesn't magically make a function execute any differently, once a function starts executing, nothing else gets to run until it finishes. Think of it like this, you need to break up doEverything() to doPart(number). When your request comes in, you do some initialization work, then call doPart(0). doPart(0) will process a few things, then queue up doPart(1) via setTimeout. Once doPart(max) is done, it will initiate returning a response to the user.

Not sure how the thread modules work in NodeJS, but from https://www.npmjs.com/package/threads it looks a lot like a webworker. In that case, it may be slower to do that way, depending on how the data gets moved around, as you essentially have to serialize everything being passed to the worker (this may not apply to Node), so you can end up in situations where you are spending more time serializing data than doing your actual work, and while the worker thread won't block, your serialize calls in the main thread will. If you can load a bunch from disk in the worker, and just return a { status: "success", result: 42 } to your main thread, will probably work out well.

Knifegrab
Jul 30, 2014

Gadzooks! I'm terrified of this little child who is going to stab me with a knife. I must wrest the knife away from his control and therefore gain the upperhand.
Hmmm maybe if I am clever I can just get postgres to do everything for me and that will solve the blocking issue as well.

Adbot
ADBOT LOVES YOU

Skandranon
Sep 6, 2008
fucking stupid, dont listen to me

Knifegrab posted:

Hmmm maybe if I am clever I can just get postgres to do everything for me and that will solve the blocking issue as well.

If using something like pgPromise, then yes, it should.

  • 1
  • 2
  • 3
  • 4
  • 5
  • Post
  • Reply