- Feature Articles
- CodeSOD
- Error'd
-
Forums
-
Other Articles
- Random Article
- Alex's Soapbox
- Announcements
- Best of…
- Best of Email
- Best of the Sidebar
- Bring Your Own Code
- Coded Smorgasbord
- Mandatory Fun Day
- Off Topic
- Representative Line
- News Roundup
- Editor's Soapbox
- Software on the Rocks
- Souvenir Potpourri
- Sponsor Post
- Tales from the Interview
- The Daily WTF: Live
- Virtudyne
Admin
Either your second hint has the wrong formula, or your article does.
Fixed - ed.
Admin
the hint does.
-100 + (x * 100)/x
Admin
How does checking the discount price for zero prevent a divide by zero when dividing by the list price?
Admin
The block where division takes place is CONDITIONED OUT :)
if(a != 0) b = 100 * a/a - 100; else b = 100;
Admin
would fix those pesky problems about representing rational numbers in floating point.
Admin
Admin
but that's not what the code above does... the code above is :
if (b != 0) b = 100 * a / a - 100
blah blah blah
Admin
Except its more like:
if (c != 0) b = a/a * 100 - 100; else b = 100;
So infact, checking for 0 doesn't in any way protect you. If I did my math right, if the discount_price is anything but 0, you get $discount = 0; otherwise you get $discount = 100 ?? That's what I'm coming up with.
Admin
(x * 100) / x = 100 so the discount will always be 0 or 100.
But why - in the first place - would you calculate discount based on the price? If their plan was that a $20 product would have a 20% discount, and a $40 product a 40% discount... I'd always buy the most expensive product (with a minimum price of $100).
The biggest WTF (I won't say real, because they are all real) is that you have no idea of knowing where the $_REQUEST value originated from. One should always use $_POST, $_GET, $_COOKIE, etc.
Admin
Yes, yes. Definitely get the list price and discount from one of $_POST, $_GET, $_COOKIE... and let me know as soon as your online store is open for business! I'd love a $0.01 shopping-spree. ;)
Admin
Better yet, store the discounts in the database, take the selected discount ID from the request and validate against the selected item(s) server-side. That way no amount of client-side data manipulation can screw you up.
Admin
$_POST/$_GET is no more secure than $_REQUEST.
Admin
Welcome to anything written in PHP!
Admin
Does the OP know what the code was meant to do? Was it really meant to be 0 or 100 discount value?
Obviously the security is a bigger WTF, because if it worked differetly then people could rob the store blind.
If I were to guess then I would put the discount calculation to be ((x-1)*100)/x
Admin
I mean if I were to guess what the code was intended to do, not what it actually does.
Admin
It seems the code is intended to figure out the percentage of discount the customer is receiving (assumedly for display purposes), given the discounted price and the original list price. Aside from all the problems with $_REQUEST and stuff, the equation should be:
(discounted_price / list_price) * 100
Unless the discounted price is $0.00 (and the list price isn't), in which case the discount should be 100%...
Addendum (2008-04-03 14:58): My mistake: ((list_price - discounted_price) / list_price) * 100
Admin
$_POST is a BIT more secure, since you can't just type random crap in the URL to mess it up.
e.g. http://www.argos.co.uk/static/Product/partNumber/7300624/Trail/C%24cip%3D1500012028.Baby%2Band%2Bnursery%3EC%24cip%3D1500012068.Clearance%2Bbaby%2Band%2Bnursery.htm
Admin
I don't have anything to say about this post, just that it's my first one since I registered and I'm happy, now I have something to put on my curriculum :) And watch! No CAPTCHA!
Admin
Well, that's only really security by obscurity. Simply don't pass data that could cause harm if changed by the user (i.e. prices) through the client and back again -- store them on the server, and only display them to the client.
Admin
Wrong Request Syntax
Correct way: $value = $_REQUEST['key'];
Request is simular to $_POST and $_GET, Depends on a forum submit type. The way accsessing is as a Single Dimension, not multiple.
And the other is that discound formula will be always zero
Admin
That's nonsense, $_REQUEST is an "array" like any other "array" in PHP (they're actually more like hashes, but whatever). It can be multidimensional no problem.
In fact, posting the following: <input name="foo[0][1][2]" value="bar" type="hidden"/> will give you a nice $_POST['foo'][0][1][2] == 'bar' var in your receiving page.
Of course, it doesn't validate against any DOCTYPE I know of (square brackets aren't allowed in name-attributes) but that's another story altogether.
Admin
Validates fine against XHTML 1.1
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN" "http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd"> <html xmlns="http://www.w3.org/1999/xhtml"> <head><title>Test</title></head> <body> <form action="/"><input name="test['field'][0][1]"/>
</form> </body> </html>Admin
Right. So a $40 item for $30 is a 75% discount, while getting the same item for $20 ($10 less) is a 50% discount. Getting it for $10 is a 25% discount. But getting it for $0... 100% discount.
Errrm...
Please inform your coworkers about this site; I look forward to some good postings!
Admin
There are at least 2 REAL problems with this code
Admin
Admin
A smack to the back of the head for both of you. POST is not security by obscurity, it's not security of any type. Using a password is an example of security by obscurity. Switching from GET to POST is like switching from 2 bullets in the gun to 1 when playing Russian Roulet with yourself.
Admin
Admin
So it's php's fault the programmer is an idiot? By that logic every language blows.
Admin
Oh well. In any case: they can be multidimensional arrays in PHP anyway. :-)
(Thanks for pointing that out to me!)
Admin
Do you not see $value = floatval($_REQUEST['list_price'][$key]);
Admin
Ah, right, sorry.
((list_price - discounted_price) / list_price) * 100
Admin
Everyone knows it should be:
if (c != 0) b = (a * 100)(7/a)(6/100); else b = 42;
Admin
Ok, some other WTFs about this that people seem to be missing:
... So the value being tested for zero isn't used in the equation regardless.
Also rather striking:
So if the discount_price is zero, ie. there is no discount, then there is a discount of... 100? Also, discounts computed in the above equation are negative, so does this mean if there's no discount_price then the price is inflated by 100 of whatever unit? WTF...?!
Dan.
Admin
You must be new here!
Admin
thinking that using $_GET, $_POST, $_REQUEST, $_COOKIE are security risks DIRECTLY shows you know absolutely nothing about the PHP language.
register_globals = on; is a security risk.
$query = "SELECT * FROM something WHERE name='" . $_REQUEST['foo'] . "'"; is a security risk
$query = "SELECT * FROM something WHERE name='" . mysql_real_escape_string($_REQUEST['foo']) . "'"; is NOT a security risk
it's not the $_-globals that are a security risk. it's MORON PROGRAMMERS who don't know when they need to do something to user input to prevent injection attacks that are a security risk.
Lot of ASP/C#/.NET/SQL Server loving trolls around. I blame Alex for writing this site in ASP (that's a WTF right there).
ASP/C#/.NET aren't so bad.. it's just they're platform-locked.
SQL Server on the other hand is a pile... coughlackinghackoffsetcoughkeywordcough
Admin
Admin
in my math universe -100 + (x * 100)/x = -100 + 100 = 0. Huge discount.
Admin
I don't really agree with that statement totally. mysql_real_escape_string() is not that hard to remember.
The part I do agree on though is that PHP is far to relaxed (same argument for VB) and so it allows people to write code that belongs on this website.
And I'm a guy who likes PHP. I just hate how lenient it is.
Admin
Admin
dillybar1: register_globals = on; is a security risk.
Register globals is only a security risk if you're not careful. It is possible to write secure code with it on.
dillybar1: So it's php's fault the programmer is an idiot? By that logic every language blows.
No, but it is is PHP's fault for making it easy for idiots to write bad code, rather than it being secure by default.
Admin
The math was supposed to be something to the effect of
but got messed up...It even would be no surprise if it was a mistake made by the submitter.
The fact that they blindly trust the user-supplied $_REQUEST might be a WTF of its own if it happens in other parts of the code, but here it likely isn't as this $discount is probably only used for display purposes.
Either way, this isn't quite worthy to be on this site.
Admin
If you don't do any division further on, then yes it doesn't.
But note that $discount = $value/value; always equals 1 except for $value equal 0 when it triggers a division/0 error. This is the error the if() protects against.
Admin
lets lose all but the last line...
Admin
I bet he or she meant:
$discount = ($discount > 100)?$list_price/$discount_price*100:100;
Admin
I believe that the original poster's point was not that $_GET et al are ALWAYS security risks -- clearly you have to get data from the user somehow -- but rather that getting something like a product price from the submitted web page is a security risk, because no matter where you think that value is supposed to come from, someone could always create his own form to submit to your site where he put whatever values he wanted in there.
(In this case the poster of the original story has explained that this was an internal web site where the users are authorized to enter whatever prices they want to anyway, so the criticism is inapplicable.)
Admin
Because they converted it to a float first....DUH. :)
Admin
I think it was supposed to be:
if($_REQUEST['discount_price'][$key] != 0) //avoid division by 0 {
$value = floatval($_REQUEST['list_price'][$key]); $discount = - 100 + ($value * 100) / $_REQUEST['discount_price'][$key];
$discount = round($discount, 2); } else $discount = 100;
So $discount is rather an additional amount of goods you get for free, than the amount of price you don't have to pay. As in commercials, when they say that you'll get 25% more of chocolate for the same money. Actually it should be $discount=INFINITY; instead of $discount=100; but normal people do not get it.
Admin
One valid definition of "security by obscurity" means something that makes it more difficult for casual users to break in, while not adding any real security.
No. There is a difference between security that uses a secret key as part of the mechanism, and "security by obscurity".
Granted, there is a sliding scale involved - rot13 is "security by obscurity" now, but thousands of years ago, it was a state-of-the-art cipher, using a secret key of "N".
Admin
Say $_REQUEST['discount_price'][$key] == 0
then
$value = floatval(0) // still zero $discount = - 100 + (0 * 100 ) / 0;
// translation: -100 + 0 / 0;
Admin
And of course if the curious math was fixed users gain the opportunity to pick any price they want by modifying the form/request string