7 ms·
> A portion of this code implemented a SMTP client. If I wanted to root cause this, the real problem is right there. Implementing protocols correctly is hard
by evmar 2y ago
> A portion of this code implemented a SMTP client.
If I wanted to root cause this, the real problem is right there. Implementing protocols correctly is hard and bugs like in the post are common. A properly implemented SMTP client library, like one you would pull off the shelf, would accept text and encode it properly per the SMTP protocol, regardless of where the periods were in the input. The templating layer shouldn't be worrying about SMTP.
- TeMPOraL 2y agoThe real problem isn't the protocol, but the cowboy approach to interacting with it. It's not hard to "accept text and encode it properly per the SMTP protocol", you just need to realize you need to do it in the first place. There is a multitude of classes of errors and security vulnerabilities, including "SQL injection", XSS, and similar, that are all caused by the same mistake that this case of missing period was[0]: gluing strings together. For example, with SQL queries, the operation of binding values to a query template should happen in "SQL space", not in untyped string space. "SELECT * FROM foo WHERE foo.bar = " + $userData; is doing the dumb thing and writing directly to SQL's serialized format. In correct code (and correct thinking), "SELECT * FROM..." bit is not a string, it just looks like one. Same with HTML templating[1] - work with the document tree instead of its string representation, and you'll avoid dumb vulnerabilities. So, if you want to avoid missing dots in your e-mails, don't inject unstructured text into the middle of SMTP pipeline. Respect the abstraction level at which you work. See also: langsec. -- [0] - And therefore should be considered as a single class of errors, IMO. [1] - Templating systems themselves are thus a mistake belonging to this class, too - they're all about gluing string representations together, where the correct way is to work at the level of language/data structures represented by the text.
- mananaysiempre 2y agoFor what it’s worth, some markup-first template systems have tried to respect the target format’s structure—Genshi[1] and TAL[2] come to mind, and of course XSLT (see also SLAX[3]). I said “markup-first” so that the whole question isn’t trivialized by JSX. [1] https://genshi.edgewall.org/wiki/Documentation/xml-templates.html https://genshi.edgewall.org/wiki/Documentation/xml-templates... [2] https://zope.readthedocs.io/en/latest/zopebook/AppendixC.html https://zope.readthedocs.io/en/latest/zopebook/AppendixC.htm... [3] https://juniper.github.io/libslax/slax-manual.html https://juniper.github.io/libslax/slax-manual.html
- spankalee 2y ago> [1] - Templating systems themselves are thus a mistake belonging to this class This is not universally true. JavaScript has an amazing feature called tagged template literals which let you tag a string with interpolations with a function that handles the literal and interpolation parts separately. This lets the tag function handle the literals as trusted developer written HTML or SQL, and the interpolations as untrusted user-provided values. Lit's HTML template system[1] uses this to basically eliminate XSS (there are some HTML features like "javascript: " attributes that require special handling). ex: html`<h1>Hello, ${name}</h1>` If `name` is a user-provided string, it can never insert a <script> or <img> tag, etc., because it's escaped. There are similar tags for SQL, GraphQL, etc. Java added a similar String Templates feature in 21. [1]: https://lit.dev/docs/templates/overview/ https://lit.dev/docs/templates/overview/
- cratermoon 2y ago> If `name` is a user-provided string, it can never insert a <script> or <img> tag, etc., because it's escaped. Be careful with that "never". A curious and persistent person might discover a bug in the implementation, leading to something like the Log4Shell issue.
- VBprogrammer 2y agoNot sure why you are being downvoted here. It's a fair point and properly escaping your data is only one part of the overall security picture but you should also be strictly validating data at the inputs to your system too.
- spankalee 2y agoLuckily, for Lit specifically, the "escaping" is done by the browser by setting textContent, so the string literally never passes through the HTML parser. Any string is valid text content, and if you found a bug that permitted unsafe text to be parsed as HTML somehow, it would be a browser bug and a very, very serious one. But it'd be similar with with other template systems. If the interpolation should allow any string, there's really no validation to be done.
- quotemstr 2y ago> Implementing protocols correctly is hard That's why it's a best practice to specify protocols at a very high level (e.g. using cap'n'proto) instead of expecting every random sleep-deprived SDE2 to correctly implement a network exchange in terms of read() and write().
- blueflow 2y agoThat why you have to read the specs of the protocol you want to implement. Its a matter of engineering rigorousness. Brute-forcing until "it works" doesn't cut it.
- Gigachad 2y agoYou don’t need to understand SMTP to send email. You just use a library that implements it and lets you just pass in a html doc.
- Joker_vD 2y agoReading is for nerds. Real programmers™ write. And they write code, for the machine, not docs or specs or any other silly stuff intended for interhuman communication.
- bregma 2y agoAh, yes, the Agile manifesto.
- vlovich123 2y agoI think you're missing the fact that experience has taught us repeatedly that separating the protocol definition from the wire format is a good idea. But sure, feel free to ignore the many lessons and blame it on individuals as if anything could be implemented free from human laziness, error, & economic demands (which btw is ironically a lack of engineering rigour which I was always taught as you assume that humans will be lazy, corrupt, make mistakes & that economics of making things as cheap as possible are a real part of engineering & not something to handwave away as "they're not doing real engineering").
- the_real_tjaart 2y agoI agree 100%.
- om8 2y agoSingle responsibility principle in its finest.
- burkaman 2y agohttps://en.wikipedia.org/wiki/Jamie_Zawinski#Zawinski's_Law https://en.wikipedia.org/wiki/Jamie_Zawinski#Zawinski's_Law
- throw10920 2y agoThe real problem is that SMTP is a "plain-text" protocol that includes in-band signaling. It literally happened because SMTP defined "a line that only contains a single period in it" as a control sequence and not a literal line that only contains a single period in it. SMTP is an example of an unnecessarily complex design, and the implementation bugs reflect it. SMTP shouldn't be hard for someone to correctly implement by themselves (even though I agree that people shouldn't be re-inventing the wheel).
- davedx 2y agoHonestly this part with the periods, while unusual, isn’t really complex. The two rules regarding the periods were like a small paragraph of text. I agree with sibling comment from Temporal, this is purely a “skill issue”, not a protocol issue
- meisenhus 2y ago[dead]
- pja 2y agoAt some point, all protocols include in-band signalling somewhere: You have to put packets on the line, and those packets are ultimately just a stream of anonymous bytes. If it wasn’t a period, it would be something else & you’d have to handle that instead.
- throw10920 2y ago> all protocols include in-band signalling somewhere That's an incredibly reductionistic view of the world that's utterly useless for anything (including actually engineering systems) except pedantry. It's obvious that the level at which you include control information is meaningful and significantly affects the design of the protocol, as we see in the submission. Directly embedding the control information into the message body does not lead to a design that is easy to implement. > If it wasn’t a period, it would be something else & you’d have to handle that instead. Yes, and there are many other design choices that'd be significantly easier to handle.
- xg15 2y agoSounds like a great idea, until you find that your SMTP library pulls in 5 other libraries as its own dependencies and those each pull in 3 transitive dependencies of their own, one being some kitchen sink/toolbox project where only 1% of its code is actually relevant to the dependant and the rest is dead weight - but which pulls in 20 more dependencies for functions that are literally never called in your project - and before you know it, your codebase bloats up by several MB and you get CVE warnings for libraries that you didn't even know existed, let alone that you're using them.
- Gigachad 2y agoThis doesn’t seem like a real issue. Servers can handle a several megabyte executable, and CVE warnings for libraries you aren’t using can be ignored.
- markisus 2y agoIn the hypothetical above, you won’t have any way to know which libraries are actually being used unless you read through the source code. Many libraries will transitively include protobuf, but most functions will not call protobuf.
- bronson 2y agoAgreed. Even if you establish that it's not being used today, that doesn't mean that it will continue to be unused after the next few commits land. And, even though you might not see a way to call into the unused code, an attacker might find a way (XZ Utils).
- andruby 2y agoI agree with this principle in general. But for SMTP libraries, that's often part of stdlib (Ruby, Python, PHP, ...).