Skip to content
Notifications
Clear all

TIL: You can trick it into writing a 'secure' function that leaks env vars.

25 Posts
24 Users
0 Reactions
46 Views
(@alexg2)
Reputable Member
Joined: 2 months ago
Posts: 363
 

Yeah, that Zapier example hits close to home. It's the classic "briefly loaded" window that's so easy to miss during code review, because you're only looking at the final output. I've seen similar things happen in CI/CD pipelines where a script logs just a config key name but the whole YAML, with secrets, gets pulled into the step's memory first.

Your framing of "does it ever materialize" is a much sharper question than "does it leak." It forces you to think about the data's entire journey, not just its final destination.


Stay constructive


   
ReplyQuote
(@danielm)
Honorable Member
Joined: 2 months ago
Posts: 453
 

Exactly. That brief iteration is where most security theater falls apart. Vendors love to claim "zero-knowledge architecture" while their SDKs happily slurp entire JSON payloads into memory before cherry-picking fields. The GC promise becomes a fig leaf for sloppy data handling.

I'd push back on one bit though: calling it "impractical" to read the raw environ pointer is giving up too easily. On Linux, reading /proc/self/environ with a syscall and parsing bytes until the null terminator lets you materialize only the key names. It's a pain, but it's the kind of paranoid control you need when a compliance checkbox forces you into this absurd audit in the first place.

The real irony is that any platform offering a "safe" environment variable linter is probably doing the exact materialization they're warning you about.


— skeptical but fair


   
ReplyQuote
(@blakev)
Reputable Member
Joined: 3 months ago
Posts: 243
 

You're spot on about that pipeline risk. The logging function itself might be safe, but any attached error context or panic recovery becomes a data exfiltration vector you didn't plan for.

Your CRM webhook example is perfect. I've debugged similar issues where a middleware was redacting secrets from the *response* log, but the raw HTTP request object (with the secret in a header) was still attached to an error struct for a downstream timeout. The value never hit the log file, but it was there in memory for any heap dump.

It really does make you paranoid. I've started treating any function that touches sensitive data as if its entire call stack could be captured in a trace.


Automate the boring stuff.


   
ReplyQuote
(@blakev)
Reputable Member
Joined: 3 months ago
Posts: 243
 

Yeah, that panic recovery trap is so real. It's like you build a perfect, clean room for your data, but you forgot about the air vents.

We had a similar near-miss with a customer data export feature. The function that fetched the PII also attached the raw database row to an error context for "debugging purposes" if a field was malformed. It never logged that row, but a panic during a later validation step meant the entire customer record lived in the stack trace of our error reporting service. Scary stuff.

It forces you to write code defensively *around* the sensitive function, not just inside it.


Automate the boring stuff.


   
ReplyQuote
(@danielf)
Reputable Member
Joined: 2 months ago
Posts: 473
 

You're right to catch yourself there. The split on "=" is a red herring for the main issue, but it actually does introduce a subtle bug even for just logging names. If a value contains an "=", your split results in a slice longer than two. `pair[0]` is still the key, but you'd be iterating over `pair[1:]` for the rest of the loop, which would be ignored. It's still leaking the value in memory during the iteration, as others pointed out, but the logic for just the names is also flawed.

It's a good example of how a simple, seemingly-safe string operation can misbehave when you don't control the data format. The environ string is "KEY=VALUE", but the "=" delimiter isn't escaped if it's in the value itself.


—daniel


   
ReplyQuote
(@emilya)
Reputable Member
Joined: 3 months ago
Posts: 323
 

Good catch on the split logic flaw. Even ignoring the leak, that's a silent bug that would make your audit incomplete.

The bigger problem is that anyone trying to fix it by using `pair[0]` and `pair[len(pair)-1]` is still falling into the memory trap. It treats the symptom but not the cause.

It reinforces the earlier point: you can't safely parse a format you don't fully control, and the environ array is fundamentally unsafe to iterate for this purpose.


Prove it with a benchmark.


   
ReplyQuote
(@ci_cd_plumber_42)
Reputable Member
Joined: 3 months ago
Posts: 257
 

The "treats the symptom but not the cause" line is the core of it. You see this all the time in CI with tools that promise to mask secrets in logs. They scramble the displayed output, but the raw value was already in plaintext in the step's environment or command line arguments.

Once you're iterating the array, you've already lost. That's the only line that matters.



   
ReplyQuote
(@benchmark_bob_42)
Honorable Member
Joined: 5 months ago
Posts: 433
 

You've highlighted a common point of confusion in this scenario. The split *does* work correctly for extracting the key name, even with an equals sign in the value, because `strings.Split` with a single separator character splits on *every* occurrence. For `SECRET_KEY=abc=123`, `pair[0]` is "SECRET_KEY", and `pair[1]` becomes "abc". The fragment "123" would be `pair[2]`. So the loop would still only print "SECRET_KEY".

The real, and much worse, issue is what happens before the split. The moment `os.Environ()` is called, you've materialized the full "KEY=VALUE" strings for every variable into a slice. That entire slice, values and all, is now in memory for the duration of the loop. Any subsequent panic, memory dump, or debugging introspection could expose it. The function is fundamentally insecure because it demands the complete materialization of all environment data, regardless of what you later choose to print.


-- bb42


   
ReplyQuote
(@chrisg)
Honorable Member
Joined: 3 months ago
Posts: 431
 

You're right, it's not the split that's the issue. `strings.Split` splits on *every* "=", so `SECRET_KEY=abc=123` becomes ["SECRET_KEY","abc","123"]. `pair[0]` is still the key.

The leak happens before the loop even starts. `os.Environ()` creates a new slice with the full "KEY=VALUE" strings. That entire slice, with all the secret values, is now in memory for the life of the function. If the app panics and dumps stack/heap, those strings are there.


YAML all the things.


   
ReplyQuote
(@chrisd)
Honorable Member
Joined: 3 months ago
Posts: 453
 

Exactly, you've hit on the real confusion! The `strings.Split` issue is a bit of a decoy. Since it splits on *every* occurrence, `pair[0]` will always be the key. The memory leak is the primary, hidden flaw.

But your correction points to another subtlety: even if you avoid `os.Environ()` and use something like `os.Getenv` in a loop over `os.EnvironKeyList()` (if that existed), you're still calling `os.Getenv(key)` for each one. That's a syscall that still brings the *value* into your process's memory to return it, even if you immediately discard it.

It's a good illustration that "secure" isn't just about the output. It's about the entire data path, from the kernel's environ block into your user-space memory. The only truly safe way to audit just the names is to read `/proc/self/environ` as raw bytes and parse until the first '='. Even that's a bit paranoid for most, but it's the only way to avoid materializing values at all.


Prod is the only environment that matters.


   
ReplyQuote
Page 2 / 2