-
Posts
4,207 -
Joined
-
Last visited
-
Days Won
209
Everything posted by Jacques1
-
Handling exceptions by loading a page with a safe message.
Jacques1 replied to ajoo's topic in PHP Coding Help
If the server indeed runs FastCGI (which any modern webserver should), try enabling the ProxyErrorOverride directive. This should intercept 500 errors and allow you to define a custom error page.- 19 replies
-
- exceptions
- errors
-
(and 1 more)
Tagged with:
-
The fact that table columns can be renamed and may contain virtually any character is exactly the problem. Sure, this attack is more difficult, because it requires a second injection vulnerability. But that doesn't mean you're safe. I wouldn't take any risks, especially when the countermeasures are very simple like in this case. One addition to #5: When you escape the backticks, make sure to use the current character encoding of the database system.
-
Stop pushing your thread. Your problems aren't any more important than the problems of other users, which means you'll have to wait until somebody helps you (or doesn't -- there's no guarantee). In the meantime, I recommend you learn how to use code tags and embed images. Most users will skip a thread immediately when they see a wall of code with no syntax highlighting.
-
If you literally set the max_post_size to “1800MB” or “200MB”, that's invalid and would explain why PHP only accepts a few bytes. It must be “200M” without the “B”.
-
You're doing it wrong. I've already explained it twice, and I'm not going to explain it a third time. If you don't care about your own application, this thread seems pointless.
-
Besides that, it's a really bad idea to delete data upon a GET request. The GET method is meant to retrieve data (hence the name), not change it. Browsers send GET requests all the time, and the user may not even be aware of it. This means there's a huge risk of data loss as well as attacks. For example, I could clear your entire sales table simply by putting a bunch of images on a website (like this one) and waiting for one of your users to come by: <img src="http://www.yoursite.com/admin_subfile_delete.php?record_id=1" alt=""> <img src="http://www.yoursite.com/admin_subfile_delete.php?record_id=2" alt=""> <img src="http://www.yoursite.com/admin_subfile_delete.php?record_id=3" alt=""> ... This would automatically delete every single record. Also, where do you check if the client is even allowed to delete records? Right now, it seems anybody can do that. I strongly recommend that you choose a more sane approach: Use sessions (or a similar mechanism) to authenticate users and make sure they actually have permission to delete records. Always use POST requests when you want to change data. That is, replace the links with forms. Use an anti-CSRF token to prevent CSRF attacks. So each form needs one hidden field for the token and one submit button. The action URL points to the specific record. // Appearently I already told you this last year, but you chose to ignore it. That's not very smart.
-
Handling exceptions by loading a page with a safe message.
Jacques1 replied to ajoo's topic in PHP Coding Help
It's a design decision of the mysqli authors. They appearently think that passing the wrong number of variables to bind_result() isn't a severe problem (I disagree), so they merely trigger a warning instead of throwing an exception. It doesn't have to be like that. An exception would actually make sense. So there are no definite rules for when to throw an exception in PHP. It depends on the person writing the code.- 19 replies
-
- exceptions
- errors
-
(and 1 more)
Tagged with:
-
That's close to 2 GiB. What kind of application is this? Try a less excessive value like 200M.
-
Your post_max_size limit is far too small (1800 bytes). Change this to something like 200M in your php.ini, restart the webserver and double-check the configuration with <?php phpinfo(); This will print all PHP settings on the screen (you should make this script password-protected so that only you can see it).
-
The above function makes the application vulnerable to SQL injection through the array keys: $table = 'test'; $values = [ 'x`) SELECT password FROM users -- ' => 123, 'x' => 123, ]; insert($table, $values); This will result in the following query: INSERT INTO test (`x`) SELECT password FROM users -- `, `x`) VALUES (:x`) SELECT password FROM users -- , :x) Dynamically generated queries are notoriously insecure and should be avoided at all costs. When they cannot be avoided, you have to religiously validate all dynamic input: Check the the field names against a whitelist of permitted column names. Do not just insert them into the query, because they may very well come from an untrusted source (e. g. the keys of $_POST). Validate the whitelisted column names. By default, only standard MySQL identifiers should be allowed. If the user needs nonstandard identifiers (e. g. column names with spaces), they should have to explicitly set a flag. Escape all backticks within the identifiers (through doubling) and then wrap the result in backticks. Put a big warning on top of the code (or into DocBlock in case it's a function). Explain the risks and recommend the use of hard-coded keys. This may sound paranoid, but a very similar problem has lead to the catastrophic Drupageddon vulnerability despite the use of prepared statements. So dynamic queries really are a huge risk.
-
Cool. The MDN has a lot of infos about data URIs: https://developer.mozilla.org/en-US/docs/Web/HTTP/data_URIs
-
OK, again: Your MakeBarcode() function returns the barcode image as a GD resource. If you want to embed this image into your page, you need to turn the GD resource into a Base64-encoded string of the PNG data. The only way to do that is to start an output buffer, save the buffer content into a variable and then Base64-encode this(!) variable: // I assume that $bar_image is the GD resource returned by MakeBarcode() ob_start(); imagepng($bar_image); $png_data_of_bar_image = ob_get_clean(); // fetch the content of the output buffer into a variable $bar_image64 = base64_encode($png_data_of_bar_image); I've verified this with different GD resources, so it definitely works. If it's somehow too difficult to implement, I suggest you go with a separate script which serves one barcode image at a time.
-
Putting the limit on top of the script is correct. However, there are a couple of limits which may be involved besides the execution time. First check the PHP error log. Does it say anything? Check the webserver configuration. Is there any limit on file uploads, POST requests or the execution time of scripts? Is post_max_size large enough? Also check where exactly the process is cancelled (e. g. by writing to a file on top of your script to see if it's even executed).
-
Make sure the script has a sufficiently high time limit.
-
You need to pass $bar_image_png to base64_encode(), not the GD resource $bar_image. When in doubt, try my minimal example above. You can still change the coding style afterwards.
-
What's the problem? If you're doing the size check with your own function, simply change the factor 1024 to 1000 so that 100M is interpreted as 100,000,000. Or actually write down “100000000” so that there's never any ambiguity.
-
Just to make sure we're on the same page: You're saying this shows the image: <?php $sample = '...'; $bar_gd_image = MakeBarcode($sample); header('Content-Type: image/png'); imagepng($bar_gd_image); But this doesn't:? <?php $sample = '...'; $bar_gd_image = MakeBarcode($sample); ob_start(); imagepng($bar_gd_image); $bar_image_png = ob_get_clean(); ?> <img src="data:image/png;base64,<?= base64_encode($bar_image_png) ?>" alt="barcode">
-
Appearently the only way to get the PNG data from a GD resource is to write it the output buffer and then read the content into a variable: <?php ob_start(); imagepng($bar_image); $bar_image_png = ob_get_clean();
-
The point is to turn the array values of $fields into keys so that they can be passed to array_intersect_key(): <?php $fields = ['username', 'pass', 'email']; var_dump($fields); $flipped_fields = array_flip($fields); var_dump($flipped_fields); Using $fields directly wouldn't work, because it has numerical keys which aren't very useful here.
-
A multipart response in the browser sounds like a very obscure approach. They do work in some browsers in some special cases (like attachments), but how do you even expect this to be rendered? Since the images are probably very small, you could embed them in an img tag with a data source: <img src="data:image/png;base64,<?= base64_encode($bar_image) ?>" alt="barcode"> Or use a separate script as suggested by QuickOldCar. If you insist on multipart messages, and if you've found a browser which can actually display them reasonable, there are some issues in your code: You need to send the first Content-Type header with the header() function. Otherwise you get plain old text/html. You're missing the terminating delimiter (with a double dash at the end). You're missing the blank line after the part headers. There are trailing spaces after your image data.
-
The array_intersect_key() stuff is a very obscure way of removing all ($key, $value) pairs from $data where $key isn't in $fields. So $fields is basically a whitelist of allowed keys (as explained in the comments).
-
Handling exceptions by loading a page with a safe message.
Jacques1 replied to ajoo's topic in PHP Coding Help
PHP takes care of all unhandled exceptions, including custom exceptions which you defined yourself. So, no, you don't need any error handling code for them. MySQLi can actually throw its own exceptions, but you have to enable this feature: <?php const DB_HOST = 'localhost'; const DB_USER = '...'; const DB_PASSWORD = '...'; const DB_NAME = '...'; const DB_CHARSET = 'UTF8'; // enable exceptions in the MySQLi driver $mysqliDriver = new mysqli_driver(); $mysqliDriver->report_mode = MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT; $databaseConnection = new mysqli(DB_HOST, DB_USER, DB_PASSWORD, DB_NAME); // try an invalid query $databaseConnection->query('THIS IS NOT SQL'); In development mode, you should now see a fatal error caused by a mysqli_sql_exception. So you don't need your own exception class for database-related errors. When you do create custom exceptions, note that you have to throw them yourself (since PHP obviously won't do it). For example: if (file_put_contents($templatePath, $template) === false) { throw new TemplateException('Failed to create template file '.$templatePath.'.'); }- 19 replies
-
- exceptions
- errors
-
(and 1 more)
Tagged with:
-
OPENSSL_RAW_DATA has nothing to do with padding. It only tells the function to use binary strings rather than Base64-encoded strings (which, for some reason, is the default). I think you're confusing OPENSSL_RAW_DATA with OPENSSL_ZERO_PADDING,. It's already impossible to crack AES-128, so increasing the key size provides no benefit. It's just slower. Encrypting all data will be extremely complicated and doesn't improve security, so, no, I wouldn't do it. Also note that the passwords still need to be hashed with a strong algorithm like bcrypt. Encryption is just an additional security layer. Storing the key in a file outside of the document root and make it readable only for the webserver is pretty much the best you can do in your case. In professional environments, keys are stored on specialized hardware security modules where they cannot be accessed directly, but this is far too expensive for a normal website. Regarding the code: Your keys are now far, far too short, because you're still confusing bytes and hex strings. An AES-128 key must be exactly 128 bits (or 16 bytes long). The hex representation is twice as long, i. e. 32 characters. Do not use 16 hex characters, because that's only 64 bits and can actually be cracked. You should store the key length in bytes in a class constant and always use that constant when generating keys. Remove all those length parameters, because they're useless and potentially dangerous. You need a lot more validation, and your error handling should be much stricter. Validate everything (input data, return values etc.), and if there's any problem, throw an exception or trigger a fatal error. Don't rely on return values, because if the caller doesn't check them, the error may go unnoticed. You should simplify your class and remove everything which isn't directly related to cryptographic operations. For example, get rid of the JSON stuff and the if-then-else magic covering dozens of special cases. The class should only do two things: Encrypt a plaintext string and decrypt a ciphertext string. Everything else belongs into a separate class. Avoid complex classes, because they dramatically increase the risk of errors.
-
The only reason for using the SKU as a key would be to store a corresponding value (or implement a hash set). Since you just want a list of SKUs, the above approach makes no sense. Yes, this kinda sorta “works”, in the same way that your array_slice() hack kinda sorta “worked”. But it makes no sense.
-
Why not simply have one PONUMB, one STRNBR and a list of SKUs? <?php $_POST = [ 'ponumb' => 'D000000034', 'strnbr' => '100', 'skus' => [ '1769057', '1768743', ] ]; foreach ($_POST['skus'] as $sku) { $update_data = [ 'sku' => $sku, 'ponumb' => $_POST['ponumb'], 'strnbr' => $_POST['strnbr'], ]; var_dump($update_data); } To get a numerical array of SKUs, use the name="skus[]" syntax.