Coding Horrors: You can gather all your technical debt into one easy framework!

The Something Awful Forums > Discussion > Serious Hardware/Software Crap > The Cavern of COBOL > Coding Horrors: You can gather all your technical debt into one easy framework!

«‹›1503 »

canis minor: May 4, 2011

down with slavery posted:

You said that it "just screams wrong wrong wrong". I'm asking why.

Because of the way variable scoping works in Javascript, there is a benefit (slash it is necessary) of being able to type var to redefine variables.

It is wrong, because it is unnecessary in this instance. If it was within different scopes, it would be arguable, but here there's no question about it.

Suspicious Dish posted:

JavaScript code:
function() {
    for (var i in foo)
        print(i);

    for (var i in bar)
        print(i);
}
This code will not do what you think it does. This is exactly equivalent to:
JavaScript code:
function() {
    var i;
    for (i in foo)
        print(i);

    for (i in bar)
        print(i);
}

I assume it's again for me - yes, that's what it will do. The point I'm making is that you'll write it the first way round, and not the second, even though it introduces the same quirky re-initialization of variable, or if you wish:

code:

var i;
for (i in foo)
   print(i);

var i;
for (i in bar)
   print(i);

Again - why would you write it like this? You'll write however `for (var i in foo)` (or, at least I do that), re-initializing the same variable i in every for. Maybe the 'wrong wrong wrong' part is something that bugs me and is wrong for me.

I apologize for taking the spotlight :ohdear:

canis minor fucked around with this message at 16:09 on Mar 6, 2014

# ? Mar 6, 2014 15:28

Adbot: ADBOT LOVES YOU

# ? May 12, 2024 04:55

Damiya: Jul 3, 2012

eithedog posted:

Again - why would you write it like this? You'll write however `for (var i in foo)` (or, at least I do that), re-initializing the same variable i in every for. Maybe it's something that bugs me and is wrong for me.

I apologize for taking the spotlight

for (var i in foo) gets optimized to

var i
for (i in foo)

regardless

so whatever

also don't use for-in

# ? Mar 6, 2014 15:40

..btt: Mar 26, 2008

Damiya posted:

also don't use for-in

Why on earth not? How else would you iterate over the members of an object of variable content?

# ? Mar 6, 2014 15:59

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

..btt posted:

Why on earth not? How else would you iterate over the members of an object of variable content?

code:

var keys = Object.keys(obj);
for (var i = 0; i < keys.length; i++) {
}

But no, `for in` is fine as long as you check the key with `Object.prototype.hasOwnProperty`.

# ? Mar 6, 2014 17:59

Steve French: Sep 8, 2003

This is at least a day old, but didn't see it posted here.

http://www.flexcoin.com

quote:

The attacker then successfully exploited a flaw in the code which allows transfers between flexcoin users. By sending thousands of simultaneous requests, the attacker was able to "move" coins from one user account to another until the sending account was overdrawn, before balances were updated.

Am I interpreting this right? Are they really *that* stupid?

# ? Mar 6, 2014 18:47

Carthag Tuek: Oct 15, 2005; Tider skal komme,
tider skal henrulle,
sl�gt skal f�lge sl�gters gang

Steve French posted:

This is at least a day old, but didn't see it posted here.

http://www.flexcoin.com

Am I interpreting this right? Are they really *that* stupid?

Yes, you are. Yes, they are.

# ? Mar 6, 2014 19:01

Steve French: Sep 8, 2003

JFC, another one!

https://bitcointalk.org/index.php?topic=499580

quote:

The hacker discovered that if you place several withdrawals all in practically the same instant, they will get processed at more or less the same time. This will result in a negative balance, but valid insertions into the database, which then get picked up by the withdrawal daemon.

# ? Mar 6, 2014 19:44

Steve French: Sep 8, 2003

quote:

The next thing that will be done--before markets are unfrozen--is a daemon will be created that continually monitors for negative balances and freezes any account with a negative balance.

# ? Mar 6, 2014 19:46

Dessert Rose: May 17, 2004; awoken in control of a lucid deep dream...

From the Department of Closing the Barn Door After the Butts Have Been Stolen...

# ? Mar 6, 2014 19:51

Steve French: Sep 8, 2003

More like closing one side of the barn door. Or closing the front door and leaving the back door open. Or closing the barn door on a barn with 3 walls.

# ? Mar 6, 2014 19:59

Dessert Rose: May 17, 2004; awoken in control of a lucid deep dream...

Oh man, I just read that post. It's pure gold from start to finish:

quote:

The hacker found a vulnerability in the code that takes withdrawals. Here's what happens when you place a withdrawal:

1. Input validation.
2. Your balance is checked to see if you have enough funds.
3. If you do, your balance is deducted.
4. The withdrawal is inserted into the database.

Well, this can't be the actual procedure, because there's no race condition here. Unless you're not locking, I suppose...

Of course you aren't. What was I thinking?

quote:

What Did Poloniex Do Wrong?

The major problem here is that the auditing and security features were not explicitly looking for negative balances.

quote:

Another design flaw is that withdrawals should be queued at every step of the way. This could not have happened if withdrawals requests were processed sequentially instead of simultaneously.

Err....

quote:

What Will Be Done to Prevent Further Exploits?

One thing has already been done: the withdrawal daemon now checks for negative balances before processing withdrawals and will freeze any account with a negative balance. This effectively prevents the exploit from being used again, but it is only a hotfix.

Okay, and then you're going to fix the race condition, right?

quote:

The next thing that will be done--before markets are unfrozen--is a daemon will be created that continually monitors for negative balances and freezes any account with a negative balance. After that, markets can be unfrozen and withdrawals resumed. Immediately following that, a daemon that will run automated audits on every account will be created, which will alert me of any strange activity and freeze any account with an overage of a balance.

Okay, and then you're going to...?

quote:

After that, withdrawals and order creation will be switched to a queued method, where the first step will be to add the task to a global execution queue that will be processed sequentially.

Right. Well, I guess not.

# ? Mar 6, 2014 20:01

ultramiraculous: Nov 12, 2003; "No..."; Grimey Drawer

Jesus Christ. I thought you guys were posting old Bitcoin exploits. Those aren't even the only [lack-of-]locking-related Bitcoin exploits in the last year. There was at least one other story a while back about another exchange (I wanna say MtGox, but maybe that's just the goto failure site in my head) that also didn't do any sort of locking and could be easily scammed on transfers if you just spammed the server with requests.

Maybe locks and transactions are fiat philosophies, or maybe all Bitcoiners are just idiots (hint: it's the latter)

# ? Mar 6, 2014 20:27

rjmccall: Sep 7, 2007; no worries friend; Fun Shoe

Processing all events on a single thread is a valid way of removing races as long as you do all your accesses via the queue and each event is individually atomic. It doesn't scale arbitrarily, but it can scale a lot better than locks if correct locking would be really expensive, e.g. if you would need to lock everything in a complex hierarchy (like a render tree in a GUI). Of course, banking transactions are not exactly that kind of case.

# ? Mar 6, 2014 20:37

evensevenone: May 12, 2001; Glass is a solid.

It's always amazed me how incredibly well-designed and written the Bitcoin protocol itself is compared to just how terrible every service that actually lets you interact with Bitcoin in any sort of meaningful way is.

# ? Mar 6, 2014 20:59

Dessert Rose: May 17, 2004; awoken in control of a lucid deep dream...

evensevenone posted:

It's always amazed me how incredibly well-designed and written the Bitcoin protocol itself is compared to just how terrible every service that actually lets you interact with Bitcoin in any sort of meaningful way is.

Two words: transaction malleability.

[Some additional words for context]

I know, let's derive the transaction identifier by hashing several key elements of the transaction. Makes perfect sense!

Let's also not define the order in which those elements are hashed.

Pinnacle of design.

But as you said... compared to the other services around Bitcoin? Yeah, it's genius.

# ? Mar 6, 2014 21:05

Munkeymon: Aug 14, 2003; Motherfucker's got an
armor-piercing crowbar! Rigoddamndicu𝜆ous.

The coiner stupidity stories have me wondering whether it's possible to structure a database such that referential integrity guarantees that an account can't be overdrawn. Probably not because it gets really hairy really quickly when I try it in my head, but I'm nowhere near an SQL expert. Plus it wouldn't help them because they probably use MongoDB because, as we all know, it's web scale.

Munkeymon fucked around with this message at 21:20 on Mar 6, 2014

# ? Mar 6, 2014 21:18

Strong Sauce: Jul 2, 2003; You know I am not really your father.

I still can't figure out if the real cause for mtgox was transaction malleability or not. I hear different things every time.

But regardless, is that problem the bitcoin protocol's fault? From what I read it was problems in mtgox's code where the rest of the bitcoin world didn't actually accept the older transactions but mtgox just accepted them without verifying.

# ? Mar 6, 2014 21:18

Munkeymon: Aug 14, 2003; Motherfucker's got an
armor-piercing crowbar! Rigoddamndicu𝜆ous.

Strong Sauce posted:

I still can't figure out if the real cause for mtgox was transaction malleability or not. I hear different things every time.

But regardless, is that problem the bitcoin protocol's fault? From what I read it was problems in mtgox's code where the rest of the bitcoin world didn't actually accept the older transactions but mtgox just accepted them without verifying.

It was their fault for assuming a transaction ID generated from a set of inputs identified the only possible transaction that could have those inputs, which was an invalid assumption as of some time in 2011.

# ? Mar 6, 2014 21:22

vOv: Feb 8, 2014

Mt. Gox is also reporting that millions of dollars worth of fiat currency was stolen, which implies that there was something besides malleability going on.

# ? Mar 6, 2014 21:29

Dessert Rose: May 17, 2004; awoken in control of a lucid deep dream...

Munkeymon posted:

It was their fault for assuming a transaction ID generated from a set of inputs identified the only possible transaction that could have those inputs, which was an invalid assumption as of some time in 2011.

Well, also you would generally assume that when you generate a transaction from your wallet, no one else can go in and get the same transaction confirmed with a different ID.

It's a really dumb bug and while, yes, clearly MtGox should have done some actual research on the protocol they were implementing before handling hundreds of millions of dollars in real and fake money, the protocol isn't really blameless either.

# ? Mar 6, 2014 22:23

Legacyspy: Oct 25, 2008

So I feel kind of proud because I've actually encountered my first "bad code."

The author was asked to create a series of bar graphs comparing some statistics across a group of cities. Some bar graphs, but not all of them, were missing cities. This was weird because we had data for those cities in the database. So I took a look at the code:

code:

bargraph1_hash = {}
bargraph2_hash = {}
bargraph3_hash = {}
cities.each do |city|
                        bargraph1_hash[city.population] = city.id
                        bargraph2_hash[city.revenue.to_f/(city.population rescue 0)] = city.id
                        bargraph3_hash[city.area] = city.id
end

# ? Mar 6, 2014 23:14

ohgodwhat: Aug 6, 2005

What's city.population rescue 0? I don't know Ruby, but it sounds like a divide by zero waiting to happen.

# ? Mar 7, 2014 00:51

xtal: Jan 9, 2011; by Fluffdaddy

ohgodwhat posted:

What's city.population rescue 0? I don't know Ruby, but it sounds like a divide by zero waiting to happen.

It's a terrible feature that rescues any exception in the preceding statement and makes it return the specified value instead. In this case, if city.population rose an exception for any reason, it would indeed divide by 0 and throw that ZeroDivisionError up the chain.

code:

>> 5/(fail rescue 0)
ZeroDivisionError: divided by 0
        from (irb):2:in `/'
        from (irb):2
        from /usr/bin/irb:11:in `<main>'

xtal fucked around with this message at 01:01 on Mar 7, 2014

# ? Mar 7, 2014 00:58

Plorkyeran: Mar 22, 2007; To Escape The Shackles Of The Old Forums, We Must Reject The Tribal Negativity He Endorsed

Fortunately rescue at least defaults to catching StandardError rather than Exception (so it doesn't catch things like syntax errors and ctrl-c).

I have no idea what that code is actually trying to achieve, though. If it was parenthesized differently it could convert divide-by-zeros into 0, which at least makes some sort of sense.

# ? Mar 7, 2014 01:27

Legacyspy: Oct 25, 2008

The whole chunk of code is riddled with issues. What brought it to my attention is that if two or more city ID's have the same value for a metric (population, area, what ever), only one of them will show up because the author is using, as keys in the hash, the metric of interest, and as value of the key, the id of the city.

It won't actually throw a division by zero error since the revenue is casted to a float, and in ruby float/0 is infinity. That will cause problems further down the line though, where he tries to plot these hashes as bar graphs.

# ? Mar 7, 2014 02:52

JawnV6: Jul 4, 2004; So hot ...

Munkeymon posted:

It was their fault for assuming a transaction ID generated from a set of inputs identified the only possible transaction that could have those inputs, which was an invalid assumption as of some time in 2011.

For that theoretical to be the case, they would have had to send out $450 million through repeated contact with their customer service. I'm willing to allow for a great deal of incompetence around bitcoin, but their customer service reps gleefully sending out their entire cold wallet is a little beyond the pale. I'm cribbing from this article.

# ? Mar 7, 2014 03:30

evensevenone: May 12, 2001; Glass is a solid.

Legacyspy posted:

in ruby float/0 is infinity.

That sounds fairly horrible in its own right.

# ? Mar 7, 2014 03:56

shrughes: Oct 11, 2008; (call/cc call/cc)

That's standard floating point behavior.

# ? Mar 7, 2014 03:59

Blotto Skorzany: Nov 7, 2008; He's a PSoC, loose and runnin'
came the whisper from each lip
And he's here to do some business with
the bad ADC on his chip
bad ADC on his chiiiiip

evensevenone posted:

That sounds fairly horrible in its own right.

It's per IEEE 754. You have to try harder to get a NaN:

Operations with a NaN as at least one operand.
Indeterminate forms
The divisions 0/0 and �∞/�∞
The multiplications 0ױ∞ and �∞�0
The additions ∞ + (−∞), (−∞) + ∞ and equivalent subtractions
The standard has alternative functions for powers:
The standard pow function and the integer exponent pown function define 00, 1∞, and ∞0 as 1.
The powr function defines all three indeterminate forms as invalid operations and so returns NaN.
Real operations with complex results, for example:
The square root of a negative number.
The logarithm of a negative number
The inverse sine or cosine of a number that is less than −1 or greater than +1.

# ? Mar 7, 2014 04:11

Jewel: May 2, 2009

Otto Skorzeny posted:

The multiplications 0ױ∞ and �∞�0

Really?? How? Doesn't "A Very Big Number", no matter how big, even if indefinitely big, multiplied by 0 give 0? 0 multiplied by anything is zero, I don't see why that should be undefined?

# ? Mar 7, 2014 04:34

pseudorandom name: May 6, 2007

JawnV6 posted:

For that theoretical to be the case, they would have had to send out $450 million through repeated contact with their customer service. I'm willing to allow for a great deal of incompetence around bitcoin, but their customer service reps gleefully sending out their entire cold wallet is a little beyond the pale. I'm cribbing from this article.

The theory is that transaction reissue was entirely automated.

# ? Mar 7, 2014 04:35

ohgodwhat: Aug 6, 2005

Jewel posted:

Really?? How? Doesn't "A Very Big Number", no matter how big, even if indefinitely big, multiplied by 0 give 0? 0 multiplied by anything is zero, I don't see why that should be undefined?

What's 0/0? What's 0*1/0?

# ? Mar 7, 2014 04:41

Dylan16807: May 12, 2010

Jewel posted:

Really?? How? Doesn't "A Very Big Number", no matter how big, even if indefinitely big, multiplied by 0 give 0? 0 multiplied by anything is zero, I don't see why that should be undefined?

Infinity is not a number, it is further than that.

Also zero in floating point behaves partially like 'indefinitely small', this is why 1/-0 = -∞

# ? Mar 7, 2014 04:42

tractor fanatic: Sep 9, 2005; Pillbug

Jewel posted:

Really?? How? Doesn't "A Very Big Number", no matter how big, even if indefinitely big, multiplied by 0 give 0? 0 multiplied by anything is zero, I don't see why that should be undefined?

https://en.wikipedia.org/wiki/Indeterminate_form

# ? Mar 7, 2014 05:32

Bognar: Aug 4, 2011; I am the queen of France; Hot Rope Guy

Jewel posted:

Really?? How? Doesn't "A Very Big Number", no matter how big, even if indefinitely big, multiplied by 0 give 0? 0 multiplied by anything is zero, I don't see why that should be undefined?

Simple example, take two functions:

f(x) = x g(x) = 1/x.

What is the value of those two functions at 0?

f(0) = 0 g(0) = 1/0 = ∞

What function represents the multiplication of those two functions?

f(x) * g(x) = x * 1/x = 1

What is the value of that function at 0?

f(0) * g(0) = 1

The value of the new function at 0, should be the same as the product of the individual functions at 0, so:

f(0) * g(0) = 0 * ∞ = 1

But what happens if f(x) = 2x? Following the same logic as above:

f(0) * g(0) = 0 * ∞ = 2

Depending on the input functions, we can make 0 * ∞ equal whatever we want it to be, and that doesn't make sense at all. 0 * ∞ is an undefined function.

# ? Mar 7, 2014 15:44

NFX: Jun 2, 2008; Fun Shoe

Sometimes I wonder if the previous developers were bored. Or maybe their ">" keys didn't work.

code:

if ((-1 < (int)eMode) && ((int)eMode < EMaxMode))
	dostuff();

According to SVN Blame, it started out as (0 < ....), which got replaced a few months later when they discovered it didn't dostuff when eMode == 0.

# ? Mar 7, 2014 17:01

qntm: Jun 17, 2009

Munkeymon posted:

The coiner stupidity stories have me wondering whether it's possible to structure a database such that referential integrity guarantees that an account can't be overdrawn. Probably not because it gets really hairy really quickly when I try it in my head, but I'm nowhere near an SQL expert. Plus it wouldn't help them because they probably use MongoDB because, as we all know, it's web scale.

One row in the table represents one minimal unit of currency. You can't have fewer than 0 rows, right? :v:

# ? Mar 7, 2014 18:07

Westie: May 30, 2013; Baboon Simulator

qntm posted:

One row in the table represents one minimal unit of currency. You can't have fewer than 0 rows, right?

If you were using this for exchanging Dogecoin, you would most likely spend more on disk space than the actual value of the currency, lol.

# ? Mar 7, 2014 18:35

HORATIO HORNBLOWER: Sep 21, 2002; no ambition,
no talent,
no chance

necrotic posted:

code:
var keys = Object.keys(obj);
for (var i = 0; i < keys.length; i++) {
}
But no, `for in` is fine as long as you check the key with `Object.prototype.hasOwnProperty`.

Just fixed a 'for in' bug this morning. Code looked like this:

code:

function findButtWithZit(zit) {
    var butts = document.getElementsByTagName("BUTT");
    var i;

    for(i in butts) {
        if(butts[i].zit == zit.pusId || butts[i].zit == zit.pusName) {
            return butts[i];
        }
    }

    return null;
}

All butts have a zit, but sometimes a zit has a pusName without a pusId (or vice versa). This happened to work fine most of the time, but on IE 8, the length property of butts was the first property to be iterated over. butts.length.zit was of course undefined, so when the pusId was also undefined, the function returned butts.length.

# ? Mar 7, 2014 19:52

Adbot: ADBOT LOVES YOU

# ? May 12, 2024 04:55

Blotto Skorzany: Nov 7, 2008; He's a PSoC, loose and runnin'
came the whisper from each lip
And he's here to do some business with
the bad ADC on his chip
bad ADC on his chiiiiip

Here's a recent one-liner bugfix:

code:

-			 xQueueSend(g_director_task, &msg, portMAX_DELAY);
+			 xQueueSend(g_director_queue, &msg, portMAX_DELAY);

Nominally, task handles and queue handles are different types, so the compiler should have caught this bug that took me hours to figure out (I guess I'm blind). The reason it didn't is that FreeRTOS re-typedefs all its internal types to pointers to void in the headers to keep people from inspecting their internals (which doesn't make a lot of sense for an RTOS to do to begin with, since people often need to have the debugger inspect the guts to reason about program behavior), preventing C's type system from warning me that I had hosed up.

# ? Mar 7, 2014 20:00

The Something Awful Forums > Discussion > Serious Hardware/Software Crap > The Cavern of COBOL > Coding Horrors: You can gather all your technical debt into one easy framework!

«‹›1503 »