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
revmoo
May 25, 2006

#basta
Hmm. I'm already using zmq I guess I could just use that. I don't really think I need a "work que" though, as I can process the data in question at once (and can't really scale past that anyway). I'm mainly concerned with "what is the fastest way to send an entire block of data to the daemon and have it picked up by whatever polling mechanism the daemon uses. I can nuke the whole data set with each update, so I'm not sure a message que is really the right tool.

I'm guessing a flat file in /tmp full of json is probably the best way to go about it.

Adbot
ADBOT LOVES YOU

bigmandan
Sep 11, 2001

lol internet
College Slice

revmoo posted:

Hmm. I'm already using zmq I guess I could just use that. I don't really think I need a "work que" though, as I can process the data in question at once (and can't really scale past that anyway). I'm mainly concerned with "what is the fastest way to send an entire block of data to the daemon and have it picked up by whatever polling mechanism the daemon uses. I can nuke the whole data set with each update, so I'm not sure a message que is really the right tool.

I'm guessing a flat file in /tmp full of json is probably the best way to go about it.

If you can nuke the data set with each update then a flat file, like you said, would do it just fine. I assumed the "integrity" of the data being pushed was important.

Experto Crede
Aug 19, 2008

Keep on Truckin'
I'm still trying to get the hang of OO, but one thing I'm stumped on is how to create a PDO object which I can use in a class that contains solely static methods.

Thus far, the only solutions I've found are:

1) Create new PDO object in every static method, which defeats the point of doing it in an OO way really
2) Don't use static methods and slap the PDO object in a constructor then instantiate the class in the file where I need it

I'm sure there's a way but I'm struggling to see what I can do here.

musclecoder
Oct 23, 2006

I'm all about meeting girls. I'm all about meeting guys.

Experto Crede posted:

I'm still trying to get the hang of OO, but one thing I'm stumped on is how to create a PDO object which I can use in a class that contains solely static methods.

Thus far, the only solutions I've found are:

1) Create new PDO object in every static method, which defeats the point of doing it in an OO way really
2) Don't use static methods and slap the PDO object in a constructor then instantiate the class in the file where I need it

I'm sure there's a way but I'm struggling to see what I can do here.

#2 is definitely the route you should be going in. For #1, you can also have static class members, but it is not the best way to do what you're wanting to do.

Blinkz0rz
May 27, 2001

MY CONTEMPT FOR MY OWN EMPLOYEES IS ONLY MATCHED BY MY LOVE FOR TOM BRADY'S SWEATY MAGA BALLS

musclecoder posted:

#2 is definitely the route you should be going in. For #1, you can also have static class members, but it is not the best way to do what you're wanting to do.

Ehhhhhhhh

You shouldn't be instantiating dependencies in your constructors because it makes your classes incredibly brittle.

What you should be doing is either having the constructor accept a PDO object or have each static method accept one and then pass one in.

karms
Jan 22, 2006

by Nyc_Tattoo
Yam Slacker

Blinkz0rz posted:

Ehhhhhhhh

You shouldn't be instantiating dependencies in your constructors because it makes your classes incredibly brittle.

What you should be doing is either having the constructor accept a PDO object or have each static method accept one and then pass one in.

I think talking about concepts like brittle classes is a bit early. When he wants to swap the pdo class with something else or add some unit testing to this class, we can talk about brittleness (and refactoring).

fletcher
Jun 27, 2003

ken park is my favorite movie

Cybernetic Crumb

revmoo posted:

Hmm. I'm already using zmq I guess I could just use that. I don't really think I need a "work que" though

¿Qué?

McGlockenshire
Dec 16, 2005

GOLLOCKS!

Blinkz0rz posted:

You shouldn't be instantiating dependencies in your constructors because it makes your classes incredibly brittle.

Correct - instead, pass through the dependency instead of instantiating it. Accepting dependencies in the constructor is a perfectly normal way to deal with this problem, and is the standard way most PHP DI containers want to work.

musclecoder
Oct 23, 2006

I'm all about meeting girls. I'm all about meeting guys.

Blinkz0rz posted:

Ehhhhhhhh

You shouldn't be instantiating dependencies in your constructors because it makes your classes incredibly brittle.

What you should be doing is either having the constructor accept a PDO object or have each static method accept one and then pass one in.

Sorry, my mistake, I misread. I immediately thought of dependency injection and read past building the PDO object in the constructor. Yes, follow what McGlockenshire said.

v1nce
Sep 19, 2004

Plant your brassicas in may and cover them in mulch.

McGlockenshire posted:

pass through the dependency instead of instantiating it. Accepting dependencies in the constructor is a perfectly normal way to deal with this problem, and is the standard way most PHP DI containers want to work.
This. For some bedtime reading, check out PHP-DI, a stand-alone PHP dependency injection container. It'll make your life easier and you can stop accessing things as statics.

Everyone's given you great advice, but I just want to throw a few whys at you. Apologies if you've heard or read this stuff already;

Experto Crede posted:

how to create a PDO object which I can use in a class that contains solely static methods.
This way lies madness. Using statics in OO is usually the exception and not the rule. If you're using a static you normally need a really good definable reason for it, and "its easier to just type Dance::doTheFunkyChicken($person, $stage);" is not a good reason.

Statics are best used when you have a dependency-less simple operation which you could have performed in-line, but you use it in so many places you've decided to make it easier to access and re-use.
Dependency-less doesn't mean argument-less, but at the same time, don't pass blatant service dependencies to static methods. That's a giant code smell which will make you hate yourself later if you don't fix it now. It's a rule of thumb that you shouldn't write bad code.

One of the biggest problems with using static methods is testability. While you can mock static methods you would need to know that the thing you're testing is going to call those methods. That means deep-inspection of the code.
With dependency injection, your classes constructor is stating a contract of requirements. It's also allowing you a place to inject mocked dependencies when testing, so functionality of your class is checked in isolation to everything else.

Not testing? Consider the need to refactor your code later on. If you make a Grapher::barGraph($data); call, then it's hard to swap out Grapher for another class because it's statically defined where it's used. If you loaded it via DI, you could just change it in the container and now $grapher->barGraph($data); goes to something else entirely.

Statics in most cases are the equivalent of traits, in that they're a way to avoid copy-pasting the same code over and over, but that's about the only feature they really provide.

Experto Crede posted:

2) Don't use static methods and slap the PDO object in a constructor then instantiate the class in the file where I need it
As Blinkz0rz and KARMA! pointed out, you don't really want to do quite that, because now your class would still end up brittle by initialising PDO internally. All you've done is moved your problem to inside the constructor rather than each method.
You still can't test it in isolation, and you can't swap out PDO. You also don't know there's a contract between your object and PDO until you look under the hood.

Instead, inject your dependency. I expect most of your code looks like this:
code:
$pants = DataAccessor::getMyPants($me);
With DI, your code would look like this:
code:
$dataAccessor = $container->get('DataAccessor');
$pants = $dataAccessor->getMyPants($me);
And your class starts to look like this:
code:
class DataAccessor
{
    /** @var PDO */
    protected $db;

    /**
     * Dependency Injection
     * @param PDO $pdo
     */
    public function __construct(PDO $pdo)
    {
        $this->db = $pdo;
    }

    public function getMyPants($me)
    {
       // ...
    }
}
And you might inject your dependency of PDO like this (see PHP-DI):
code:
return [
    'db.dsn'    => 'my:dsn:goes:here',
    'PDO' => function (Container $c) {
        return new PDO($c->get('db.dsn'));
    },
    'DataAccessor' => DI\object()
        ->constructor(DI\get('PDO')),
];
What do you get for this effort?

With your DataAccessor, the __construct outlines a contract about what DataAccessor needs before it can be created.
Because you've got DI, you can now test DataAccessor in isolation; you can inject a mock of PDO and know that you're only testing the logic within DataAccessor and you'r not touching the DB or anything like that.
Using a container like PHP-DI, the DataAccessor (and PDO!) will only be initialised once per run of your application. It's faster and less memory intensive.

Edit: don't trust my PHP-DI code. This was just a 5 minute example.

v1nce fucked around with this message at 06:06 on Nov 13, 2015

v1nce
Sep 19, 2004

Plant your brassicas in may and cover them in mulch.
I like this thread, so I will post more things.

I've been lumped with the job of creating an API for one of our systems, in like, a day.. but gently caress me, right?
Anyway, I figured I'd use one of the existing Symfony bundles to get this job done quickly and save myself from reinventing the wheel.

My goals were:
- JSON support, XML if possible
- REST-ful style maybe HATEOAS-esq, probably following HAL rather than jsonApi so other people can pick it up quicker
- Able to handle some RPC garbage because we have a lot of old AJAX poo poo we want to put under this umbrella and then fix.
- Immutable endpoints, incrementing with versions.

I found three good candidates:
https://github.com/FriendsOfSymfony/FOSRestBundle
https://github.com/willdurand/BazingaHateoasBundle
https://github.com/GoIntegro/hateoas/

But here's the thing, all three of these bundles use annotations on the Entities (some hijacking the Doctrine class meta) rather than defining an actual resource. Now, I get that's a poor-mans quick way to do a REST api, but did we all just happen to forget about MVC because of convenience? I couldn't find a clear cut way to override this and feed my own Resource definitions and serialiser, so I'm having to roll my own.

GoIntegro was almost there with the Muggle controllers, but their serialiser is still based off the entity annotations. It deceptively mentions stuff like "version: v1" in the routing, but everything is reliant on that entity definition. If you change your poo poo, your API goes walkabout, so whats the point?

BazingaHateoasBundle with FOSRestBundle has some good stuff going for it, but it's all tightly coupled (at least in the compatible version) to JMSSerialiserBundle, which is like a bull in a china shop with my data. Plus I can't limit the output, I have to put up with whatever JMS decides to do to my entity.

So I'm stuck rolling my own Resource definition, so make the resource data output immutable, my own serialiser, to make the entity serialisation reliable and standard format across all Resouces, and I'm doing all this inside FOSRestBundle and then throwing it at BazingaHateoasBundle and hoping poo poo doesn't explode.

Am I just missing something?
Who decided it was OK to put your View layer in your Model layer?
Why aren't these bundles built for versioning?

musclecoder
Oct 23, 2006

I'm all about meeting girls. I'm all about meeting guys.
I was in the same boat as you, I just ended up writing my own API stuff. None of the bundles did completely what I wanted. However, I did stick with the JMSSerializer (however, I don't like how it just ignores a field that's null).

You can build a basic API out of the gate with just standard Symfony components, use HTTP Basic Auth against API keys in your database, use a Listener to make every response using the Content-Type you want (and another Listener to force the Accept or Content-Types of the request), and do simple versioning with the URL like /api/v1/resources.

Especially if this is an internal API or something basic - you're not building Stripe or Twilio.

v1nce
Sep 19, 2004

Plant your brassicas in may and cover them in mulch.
Thanks, it's good to know I'm not going insane. I also tried crowbarring Apigility into the system, but ran into too many hurdles to get it running, which is a drat shame.

Internally sure, we can cut some corners, but I've seen way too much internal code get re-purposed for client use, so I don't really want to let people (including me) run wild in an unconstrained system.
I'd rather people complain it's long winded to set up than they screw up the API for a client-facing system that interacts with their payroll provider, or have entity changes puke sensitive data out the endpoints because they used a field blacklist instead of a whitelist.

If someone meddles with the entities, ideally all I have to modify for existing endpoints not to poo poo the bed is my Normalizer.

edit: I keep saying serializer instead of normalizer because I'm stupid.

v1nce fucked around with this message at 00:57 on Nov 25, 2015

teen phone cutie
Jun 18, 2012

last year i rewrote something awful from scratch because i hate myself
I'm having a problem with telling if my function to negate SQL injections is working correctly or not.

The point of it is to get rid of the slashes that end the query and then replace them back because this is a dictionary application and I'd like to accept all characters.

This is what my functions to clean the input look like:



and here's my queries to input a word and definition look like:



I ran this add_word function with and without cleaning and I got two results. The first circled row is with the clean function but I cannot tell if it's still possible to harm my database or not. I'm assuming the second row is a unwanted result, but I'm not really sure.



How do SQL injections even work? I really don't understand them that well.

teen phone cutie fucked around with this message at 05:18 on Nov 25, 2015

Acer Pilot
Feb 17, 2007
put the 'the' in therapist

:dukedog:

Don't do it! Use prepared statements/PDO/whatever they call it now.

v1nce
Sep 19, 2004

Plant your brassicas in may and cover them in mulch.
Stop putting quotes around everything. The fact you've got ->clean_array('$word') is your problem.
Single-quotes don't get interpreted, so $word is literally the string $word and not a variable. If you just do ->clear_array($word) it'll be handled as a var.

What IDE are you using? It's highlighting is unhelpful. Notice that every $something is light-blue? that means it's a variable. Where you've got '$word' it's grey, meaning it's a string. Your IDE is lovely though, because "$word" will be interpreted as a variable, but your IDE thinks it won't.

Here's some non-lovely highlighting (blur added care of my Surface):


There's no example of "hello my name is $name" in there, but that's mostly because I couldn't find any examples of it in my code.

SQL Injection and You

If you're doing this for anything besides academia, as in, you're doing SQL injection proofing to protect against actual SQL injection attacks and not just to stop your code crashing when you put in "O'neil".. basically like, if it's ever going to be put on a public-facing server and you don't want to lose your data then this advice applies to you.

SQL injection is literally where someone puts SQL into the input data (url, form, whatever) and your server is stupid enough to execute it. What's an example of this?
code:
$carModelName = $_REQUEST['car_model_name'];
$cars = $qb->query("SELECT * FROM cars WHERE model = '$carModelName'");
So what's wrong here? well normally you'd put "ford" or whatever into car_model_name and your SQL statement becomes:
code:
SELECT * FROM cars WHERE model = 'ford'
But if I put ';DROP TABLE cars;-- it becomes:
code:
SELECT * FROM cars WHERE model = '';DROP TABLE cars;--'
What'll that do? It'll search for nothing, then it'll drop the table. Two statements for the price of one. Yay! SQL injection.

And it's not just an actual SQL Injection attack that'll show up this problem. If I put "O'neil" into $carModelName then the database will panic with a malformed SQL error because my statement has become:
code:
SELECT * FROM cars WHERE model = 'O'neil'
Notice there's one too many single-quotes in there? Yeah. SQL sees model = 'O' and then it's like "what the gently caress is neil'?"

how 2 fix
Things people think they can do, but they won't work:

Restrict the DROP rights on the MySQL user.
That's a nice thought, but DROP is only one attack of many potential vectors. Allowing SQL injection at all is a no-no.

Escape the data before it hits your statement.
This was the old classic way of doing it and is why the horror of Magic Quotes even exists. It's also the approach you're taking.
Problem is, you can do SQL injection with a massive array of vectors - some easily detected, some really stupid hard to differentiate from legit data.
This is a crappy solution which requires you to gently caress with data you shouldn't have to gently caress with.

What you really need to do is separate the SQL and the data. In comes prepared statements to save the day.

MySQLi, which I think is what you're using, supports this (barely):
code:
$stmt = $mysqli->prepare("SELECT * FROM cars WHERE model = ?")) {
$stmt->bind_param("s", $carModelNAme);

/* execute query */
$stmt->execute();

/* bind result variables */
$stmt->bind_result($cars);

/* fetch value */
$stmt->fetch();

/* close statement */
$stmt->close();

return $cars;
Holy crap, look at all that horse poo poo. This is why nobody likes MySQLi. Where did $cars come from? Why so many statements? Oh god, so much work. I miss mysql_query.

Rock on PDO, a lot less annoying version of the same thing.
code:
$stmt = $db->prepare('SELECT * FROM cars WHERE model = :carModelName');
$stmt->execute(array(':carModelName' => $carModelName));
$cars = $stmt->fetchAll();
Magical. This tells PDO that the :carModelName key will be swapped out with the $carModelName. It does all the data escaping and whatever inside PDO and the DB, so you don't have to worry about it.

With this set up, we don't need magic quotes enabled and you can stop parsing all your data. You've just simplified your code and removed tons of lines of stuff that didn't work anyway. You're welcome.
So we don't have to worry about SQL Injection ever again, right?

Wrong. Let's pretend you're having an off day and this tumbles out your brain:
code:
$stmt = $db->prepare('SELECT * FROM cars WHERE model = "'.$carModelName.'"');
$stmt->execute();
$cars = $stmt->fetchAll();
Boom, that stupid concatenation of the variable inside the statement puts us right back in SQL Injection land, and people are Timmy drop tabling your poo poo left right and center.

What's the key take-away from this? Don't mix your SQL and your data. Like, ever. If you can't put a wall between your statement and your data then you are doing it wrong.

Why does it matter so much? Because big companies keep loving it up.

Bonus extra: Separating your data and your statement doesn't mean you can't have logic to build your statement. This is totally legitimate:
code:
/**
 * Search cars by Model and Manufacturer.
 * Returns nothing if no search params are given. Finds all results in either filter.
 *
 * @param string $carModelName Name of car model to search for
 * @param string $manufacturerName Manufacturer name to search for
 * @return array Cars found
 */
function findCars($carModelName, $manufacturerName)
{
    // Get everything from cars, but by default get no results (1=2 is impossible)
    // unless a filter is given too.
    $sql = "SELECT * FROM cars WHERE 1=2 ";
    $vars = [];

    // Filter: car model name
    if (strlen($carModelName)) {
        $sql .= " OR model = :carModelName";
        $vars[':carModelName'] = '%'.$carModelName.'%';
    }

    // Filter: manufacturer
    if (strlen($manufacturerName)) {
        $sql .= " OR manufacturer LIKE :manufacturerName";
        $vars[':manufacturerName'] = '%'.$manufacturerName.'%';
    }

    // Do statement
    $stmt = $db->prepare($sql);
    $stmt->execute($vars);
    return $stmt->fetchAll();
}
Notice the statement goes in $sql, and the data goes in $vars. Don't cross the streams. It would be bad.

v1nce fucked around with this message at 06:33 on Nov 25, 2015

Heskie
Aug 10, 2002

v1nce posted:

Useful SQL stuff

Sorry for the de-rail from all the awesome info, but which color scheme is that?

karms
Jan 22, 2006

by Nyc_Tattoo
Yam Slacker

Heskie posted:

Sorry for the de-rail from all the awesome info, but which color scheme is that?

It's not monokai, so who cares? :skeltal:

v1nce
Sep 19, 2004

Plant your brassicas in may and cover them in mulch.
/\/\/\ :bahgawd:

Heskie posted:

Sorry for the de-rail from all the awesome info, but which color scheme is that?
It's custom. I think some other editor (PHPEdit? UltraEdit?) had these styles back in the day and they've just stuck with me as the most usable. Here's a gist of the ids file if you're interested.

Heskie
Aug 10, 2002

v1nce posted:

/\/\/\ :bahgawd:

It's custom. I think some other editor (PHPEdit? UltraEdit?) had these styles back in the day and they've just stuck with me as the most usable. Here's a gist of the ids file if you're interested.

Awesome, thanks for this. I use and like Solarized Dark at the moment, but yours looks pretty cool.

v1nce
Sep 19, 2004

Plant your brassicas in may and cover them in mulch.
Just in case anyone's interested, the way I ended up going with the API uses FOSRestBundle for the service-architecture alone. I'll probably replace that, too, with a custom solution soon enough.

A really simple controller example:
php:
<?
class UserApiController extends BaseApiController
    public function userGetAction(Request $request, $id)
    {
        $em = $this->getDoctrine()->getEntityManager();
        $user = $em->getUserRepository()->find((int) $id);

        // Validate: user must exist
        if (!$user instanceof User) {
            throw new NotFoundHttpException();
        }

        // Display the User
        return $this->viewResource(new UserResource(), $user);
    }
}
?>
That viewResouse method generates a custom "ApiResourceResponse" object, and a listener for KernelEvents::VIEW picks it up and deals with it.

The UserResource is the immutable API structure, so it looks a bit like this:
php:
<?
class UserResource extends AbstractResource
{
    /**
     * @inheritdoc
     */
    public function buildResource(ResourceBuilder $builder)
    {
        // Resource props
        $builder
            ->setTypeName('user')
            ->setClassName(User::class)
            ->setNormalizer(new UserNormalizer());

        // Fields
        $builder
            ->addIdentifier('id')
            ->add('first_name', 'firstName')
            ->add('last_name', 'lastName')
            ->add('email_address', 'email')

        // Relation: Employees
        $builder->addManyRelation('team_members', 'employees', ResourceBuilder::READ_ONLY)
            ->setResource(new UserStubResource());
    }
}
?>
The fluent interface gives us another level of abstraction in case I've really screwed the pooch in my design somewhere, plus it gives my devs some idea of what options are available when they make their own resources.
The normalizers are just your regular symfony normalizers (but support callbacks in both directions), and it means if our entities change shape all we have to do is re-align the normalizers to make everything backwards compatible.

It's also got circular reference protection, request parsing, error handling, read-only fields (hacky quick fix for now), and you can pick and choose the normalizer or sub-resource classes you want to use. This avoids the problem of not knowing what the normalizer is going to do, and lets us do progressive upgrades of each endpoint, rather than having the whole API (and serialisers) need to jump version at once.

In the view serialiser I ended up with something that looks very similar to ROAR (ruby) without even knowing it existed.
With a resource definition and a entity I can now generate/handle flat JSON, JSON-HAL, JSON-API or XML by just changing the Accept/Content-Type headers, and the main serializer switches to the appropriate handler class and generates/parses the data.

I really hate rolling my own, especially something as complicated and as hairy as this, but the plus side is I've ripped off all the best features of several different bundles and throw out a ton of cruft. The new methods are backwards compatible with our RPC POX/JSON garbage and the whole thing has proper MVC.

I also discovered API Platform, which is like the ultimate API implementation by some core symfony dudes, but it's only 2.7 compatible, so completely useless to me. It has the concept of Resources (though not the way I'd do it) which can probably be used to achieve versioning, while everything else is dynamically generated off entities AFAIK.

Edit: for API Platform.

v1nce fucked around with this message at 23:03 on Nov 30, 2015

musclecoder
Oct 23, 2006

I'm all about meeting girls. I'm all about meeting guys.
Looking good, v1nce. I would make the suggestion that you look into Symfony's ParamConverters to remove the boilerplate code of searching for an entity and throwing a 404 exception if it's not found (not sure if you're doing that or not since the controller is just an example).

v1nce
Sep 19, 2004

Plant your brassicas in may and cover them in mulch.
Thanks, and yeah that's a good shout and I'm sure you mentioned them once before, but the whole system is so legacy I've just straight up avoided going against the established "standard" for the time being.

Maybe once I have a proper test framework plugged in I'll look at changing peoples ways :stonkhat:

stoops
Jun 11, 2001
Okay, so i know php, but i'm not that great. I know how to get by though. My problem stems that i never really took a programming class, so I don't know proper industry standards and whatnot.

Now that that's out of the way, a website I was working on got audited and I need to address the security concerns.

I have a form that can upload files.

When the auditors tried the url http://website.com/uploads/, they received an Access Forbidden page, which confirmed to them that a directory did exist.

Is there any way i can prevent this Access forbidden message to come up? Or, how do I address this?

Any help is appreciated.

revmoo
May 25, 2006

#basta
Assuming no framework--you can either have PHP throw a 404 and then have your server configured to serve out a 404 page, or you can do it with a .htaccess file.

McGlockenshire
Dec 16, 2005

GOLLOCKS!
The fact that they can see that /uploads/ exists in and of itself is a trivially minor security problem, but it being detectable might give attackers clues that there's a file upload mechanism somewhere that they might be able to attack.

Can you tell us more about the upload mechanism? What does it do? What's it for?

Based on those answers, we can give you actual advice on security. Simply making the directory undetectable is more of a workaround than anything else.

stoops
Jun 11, 2001

McGlockenshire posted:

The fact that they can see that /uploads/ exists in and of itself is a trivially minor security problem, but it being detectable might give attackers clues that there's a file upload mechanism somewhere that they might be able to attack.

Can you tell us more about the upload mechanism? What does it do? What's it for?

Based on those answers, we can give you actual advice on security. Simply making the directory undetectable is more of a workaround than anything else.

The uploads are for documents and/or images.

A user fills out a form (that ends up in a database), but they can also attach a document which goes in the uploads directory. The file is renamed to md5 filename when its in the uploads directory.

McGlockenshire
Dec 16, 2005

GOLLOCKS!
What kind of document type detection to you use to ensure that only valid documents are uploaded? Remember, never trust the user, never trust the browser. It's easy to upload something that might appear to have the MIME type application/pdf but really be, say, a PHP script in disguise. Or maybe Javascript. Or maybe just HTML. All the user needs to do is figure out how to upload content that might either trick the server into running things, or trick the browser into running things.

That's the biggest threat you face.

The second threat is that if the user can guess what the filename is going to be, they can go guessing for things in /upload/ that they might not otherwise have access to. Instead of just plain md5ing the original filename, use hash_hmac with a longish key and a stronger algo. This will increase the difficulty of finding other documents.

The third threat is the presence of these documents inside the web root at all. Is it important that any user be able to grab any document at any time, unauthenticated? If only authenticated users should be able to retrieve documents, then the documents should live outside the web root and you should use PHP (or anything else, honestly) on the server-side to perform access control. An alternative might be HTTP authentication, but don't ever use it - not even in digest mode - without SSL.

v1nce
Sep 19, 2004

Plant your brassicas in may and cover them in mulch.
I'm only re-iterating what McGlockenshire said but with different words.

There are two types of files; public and private.

Public files are files you don't mind being downloaded by anyone. A member of the public can come along, get the file, no problem.
These types of files can sit in your web root and users can browse to them without any kind of security concerns getting in their way. The file names can be human readable (my_awesome_file.doc), the list might have an index, etc, etc.

Private files on the other hand are files you don't want joe public to have access to. This might be things like CVs, certificates, licenses, whatever.
These files should ideally sit outside of your web root. This means nobody can ever browse to them directly, and prevents a mis-configured server from granting people access to your poo poo.
There should also be some kind of ACL (Access Control Layer) which sits between the clients request for the file, and them receiving the file. This lets you authenticate them and ensure only authorised users have access.

How you store the file is up to you. How you do ACL is up to you.

Side note: you can also use the Private system to house public files, you just have an exception in your ACL which skips any authentication for files marked as public.

The problem with storing md5 files in /uploads/ with no ACL is the only security you've got is security-by-obscurity.
Security by obscurity does not provide any actual security - you're not plugging any holes - it simply obscures weaknesses and makes it more difficult for an attacker to identify a vector. You need a security system - an ACL - if you want to provide security and not just hide them.

If you change your /uploads/ directory from a 403 to a 404 you're just adding another layer of security-by-obscurity, which is a bit of a dumb suggestion by the auditor, rather than telling you your secure files aren't actually secure at all.
Obscurity does nothing to prevent an unauthorised person - who happens to know the URL of another persons files - from downloading their poo poo.

But how would someone get the URL? Is this worth worrying about? you say. Yes, there are lots of ways to get the URLs someone else visited. Yes, it's a real vulnerability. It's worth worrying about to the value you and your clients put on the documents you're keeping secure.
Don't think they're that valuable? Think about the impact it will have on your users if those documents are discovered to be insecure, and the questions they will then place upon the rest of your system, and your capabilities.

If you provide us a little more information about your setup (no framework, php, mysqli/pdo, how do you manage users?) we can give you more details about the code you need to do all of the above.

stoops
Jun 11, 2001

Thanks for the info.

I'm using the cakephp framework. I do have an acl system going on, where a user has to login, i set permissions, and whatnot...BUT I did store all the files on webroot/uploads, so, yeah, anyone that wasn't logged in but knew the url can grab the file.

Currently, I've created a folder that's not under webroot to house my files. I've been going thru the cakephp docs and I think i've got it where nobody can get to the files. If I have this working correctly, should i still rename the file uploaded?

As for how it's getting uploaded, I'm using a File Uploader Component that was already in place.

I do all my validating on the controller.

basically:

- file gets uploaded,
- i check if it's a txt or doc, or some type of image,
- if its not, i delete the file


I'm using cakephp. apache. Php version is not the latest; it's probably a little before that, 5.4 i think, not 5.8. i use mysql

substitute
Aug 30, 2003

you for my mum

stoops posted:

Thanks for the info.

I'm using the cakephp framework. I do have an acl system going on, where a user has to login, i set permissions, and whatnot...BUT I did store all the files on webroot/uploads, so, yeah, anyone that wasn't logged in but knew the url can grab the file.

Currently, I've created a folder that's not under webroot to house my files. I've been going thru the cakephp docs and I think i've got it where nobody can get to the files. If I have this working correctly, should i still rename the file uploaded?

As for how it's getting uploaded, I'm using a File Uploader Component that was already in place.

I do all my validating on the controller.

basically:

- file gets uploaded,
- i check if it's a txt or doc, or some type of image,
- if its not, i delete the file


I'm using cakephp. apache. Php version is not the latest; it's probably a little before that, 5.4 i think, not 5.8. i use mysql

Moving the uploads folder out of webroot should be good enough, since the default .htaccess files in Cake (and your Apache config) should already limit public web access to only the webroot folder.

I'm guessing you add the file name (original and/or md5 name) into a DB table linked to the user.

Are you allowing access to the uploaded files to authenticated users? If you need to do that, there are ways to serve the files back to a user from the non-public folder.

revmoo
May 25, 2006

#basta

quote:

Currently, I've created a folder that's not under webroot to house my files. I've been going thru the cakephp docs and I think i've got it where nobody can get to the files. If I have this working correctly, should i still rename the file uploaded?

Yes, because what happens if someone uploads a file with the same name as a previous file?

Also don't just md5 it because you'll have the same issue, md5+microtime() should be fine.

Loving Africa Chaps
Dec 3, 2007


We had not left it yet, but when I would wake in the night, I would lie, listening, homesick for it already.

I'm having some trouble with a study i'm planning using PHP. The aim of the study is to look at how drug labelling can effect error rate and to do this i've written a web page that displays between 5 and 8 randomly selected drug ampules that i've drawn and asks people to select the name one within three seconds. The issue i'm having is while the drug selection seems to be random the shuffling of the array doesn't. People i've shown the project to have noticed, for example, that if naloxone is one of the drugs it's always displayed furthest right.

code:
$numvials = mt_rand(5, 8); //set the number of drugs to use

$cvial = mt_rand(1, $numrows); //randomly select one vial to be correct one (mersene twister)
$vials[] = $cvial; //add correct vial to initilise array

while(count($vials)<$numvials){ //while we haven't complete our set of drugs
	$newvial = mt_rand(1, $numrows); //select a random vial
	if (in_array($newvial, $vials)){
		//do nothing as already selected
	} else {
		array_push($vials, $newvial); //add it to array
	}
}

shuffle($vials); //shuffle the order
$qvials = "'". implode("', '", $vials) . "'"; //implode array ready for query

mysql query pulls relevant rows and code then loops to display pictures and populate a form. This is the point that i think must be the problem. It looks like the query returns the drugs in numerical order of their ID hence it's no longer random. Is ORDER BY RAND() rather then shuffling the array the best way to fix this?

Loving Africa Chaps fucked around with this message at 19:21 on Dec 3, 2015

Impotence
Nov 8, 2010
Lipstick Apathy
out of curiosity, why are you using mt_rand?

Loving Africa Chaps
Dec 3, 2007


We had not left it yet, but when I would wake in the night, I would lie, listening, homesick for it already.

Biowarfare posted:

out of curiosity, why are you using mt_rand?

When looking for what function to use the page on rand() on php.net seemed to suggest it wasn't as random as it should be and that mt_rand() was the superior choice

Begby
Apr 7, 2005

Light saber? Check. Black boots? Check. Codpiece? Check. He's more machine than kid now.

Loving Africa Chaps posted:

When looking for what function to use the page on rand() on php.net seemed to suggest it wasn't as random as it should be and that mt_rand() was the superior choice

It should work for this fine.

I have no idea what shuffle is doing, never used it. You can work around it though. How about instead you just randomly select some vials, then after you are done select which of those is the correct one? So if you have 4 vials in $vials you could do

php:
<?
$cVialIndex = mt_rand(1, count($vials)); 
$cVial = $vials[$cVialIndex);
?>
Actually you could just rewrite the entire thing something like
php:
<?
$vials = array();
$numVials = mt_rand(5, 8);

while(count($vials) <= $numVials)
{
   $vial = mt_rand(1, $numRows);
   if(!in_array($vial, $vials)) $vials[] = $vial;
}

$cVialIndex = mt_rand(1, $numVials); 
$cVial = $vials[$cVialIndex];
?>

substitute
Aug 30, 2003

you for my mum
PHP 7 officially released today.

http://php.net/archive/2015.php#id2015-12-03-1

v1nce
Sep 19, 2004

Plant your brassicas in may and cover them in mulch.
It took me a while to figure out that the aim of your code was to generate a series of randomised IDs for use in a SELECT statement.

The order of the IDs you use in your WHERE query has nothing to do with the order they will be selected in. The return order is the order you specify in the queries ORDER BY, and if you specify no ORDER BY it'll be ordered by the primary key (the ID).

Also, here's a quicker way to generate those numbers and randomise the sequence
code:
// Create array of 1-n and shuffle order
$vials = range(1, $numrows);
shuffle($vials);

// Select one at random as the correct one (this makes no sense to me?)
$cvial = array_rand($vials);

// Trim array to first 5-8 numbers
$quantity = rand(5, 8);
$in = array_slice($vials, 0, $quantity);

// Make IN values
$qvials = implode(', ', $in);
The problem with your use of while() is it'll do a lot of wasted cycles until mt_rand finally delivers a value that isn't already in the array. You can instead just generate a range of values in an array, shuffle it, then trim it.
Your code would be more efficient if you had a large result set with an unlikely number of ID collisions. But you don't, so it's not.

You can make SQL randomise results, but the problem is that it doesn't scale well - the more items in your table then the worse the performance.

So instead, let's keep using your random number selection, and just randomise the results returned by SQL.
code:
// Let's pretend you're using PDO
$stmt->execute();
$results = $stmt->fetchAll();
shuffle($results);

poxin
Nov 16, 2003

Why yes... I am full of stars!
I have been wracking my brain on this one, nothing seems to work and I'm not sure what is being overlooked.

I have an array that I am pulling from a (crap)API, it looks like:

code:
Array
(
    [title] => Servers
    [servs] => Array
        (
            [0] => Array
                (
                    [server_name] => localhost
                    [total_ram] => 15922
                    [ram] => 12844
                    [total_space] => 931
                    [space] => 263
                    [version] => 2.7.9
                    [numvps] => 4
                    [alloc_ram] => 5120
                    [alloc_space] => 45
                    [virts] => Array
                        (
                            [0] => kvm
                        )

                )

            [1] => Array
                (
                    [server_name] => VS999
                    [total_ram] => 15922
                    [ram] => 5897
                    [total_space] => 1397
                    [space] => 974
                    [version] => 2.7.9
                    [numvps] => 4
                    [alloc_ram] => 10240
                    [alloc_space] => 200
                    [virts] => Array
                        (
                            [0] => kvm
                        )

                )
Using this print_r in PHP it generates that just fine.
code:
$output = $admin->servers();
print_r($output);
I'm trying to pull out specific values from that array and loop them into a grid (using vue.js) which looks like this:

code:
{ Node: 'vs999', Disk_Free: 974, Total_Disk: 1397, Disk_Free_Percent: 69, Free_RAM: 5897, Total_RAM: 15922, Free_RAM_Percent: 37, VMs: 4, Version: '2.7.9' },
Is there any easy way to accomplish this? I've tried printing single values from the array itself such as "server_name" but I can't get that even working - just a blank page no matter what I try.

Adbot
ADBOT LOVES YOU

Loving Africa Chaps
Dec 3, 2007


We had not left it yet, but when I would wake in the night, I would lie, listening, homesick for it already.

v1nce posted:

It took me a while to figure out that the aim of your code was to generate a series of randomised IDs for use in a SELECT statement.

The order of the IDs you use in your WHERE query has nothing to do with the order they will be selected in. The return order is the order you specify in the queries ORDER BY, and if you specify no ORDER BY it'll be ordered by the primary key (the ID).

Also, here's a quicker way to generate those numbers and randomise the sequence
code:
// Create array of 1-n and shuffle order
$vials = range(1, $numrows);
shuffle($vials);

// Select one at random as the correct one (this makes no sense to me?)
$cvial = array_rand($vials);

// Trim array to first 5-8 numbers
$quantity = rand(5, 8);
$in = array_slice($vials, 0, $quantity);

// Make IN values
$qvials = implode(', ', $in);
The problem with your use of while() is it'll do a lot of wasted cycles until mt_rand finally delivers a value that isn't already in the array. You can instead just generate a range of values in an array, shuffle it, then trim it.
Your code would be more efficient if you had a large result set with an unlikely number of ID collisions. But you don't, so it's not.

You can make SQL randomise results, but the problem is that it doesn't scale well - the more items in your table then the worse the performance.

So instead, let's keep using your random number selection, and just randomise the results returned by SQL.
code:
// Let's pretend you're using PDO
$stmt->execute();
$results = $stmt->fetchAll();
shuffle($results);

The correct vial statement is because one drug is the one people have to click out the of the selection the script produces and i'm looking at the time it takes and error rate to see if different ways of labelling the drugs makes a difference to error rates. Here's what i've got so far which might better explain it (timer is currently slowed down massively to let me play around with stuff) http://huzenhagen.net/.. This looks great though i'll see if it improves things

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